Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use window.top.location instead of window.location when embedded in iframe? #3950

Closed
bhousel opened this issue Apr 6, 2017 · 5 comments
Closed
Labels
core An issue with one of the core iD components

Comments

@bhousel
Copy link
Member

bhousel commented Apr 6, 2017

see openstreetmap/openstreetmap-website#1518

The issue here is that on openstreetmap.org, iD is running in an iframe. We detect the host using this line of code.

window.location.origin + window.location.pathname
writes "https://www.openstreetmap.org/id" into the changeset host tag.

When embedded, we could do:
window.top.location.origin + window.top.location.pathname
to get "http://www.openstreetmap.org/edit" in the changeset host tag.

@bhousel bhousel added the core An issue with one of the core iD components label Apr 6, 2017
@HolgerJeromin
Copy link
Contributor

Perhaps id should check if all needed libraries are loaded and abort gracefully with a message in the body. This could contain a link to the osm main page.

@HolgerJeromin
Copy link
Contributor

You said

You're right that window.location when embedded in an iframe is not the true location.

But in a way that is the true location. /edit can give multiple editors depending on the user preference.
Perhaps under this URL there could be id version 5 under a iframe /iD and a version 10 under /id10
Putting /edit in the tag would not differ both. But the version tag would...

But still we had the problem with old URLs in the DB right now

@bhousel
Copy link
Member Author

bhousel commented Apr 7, 2017

#2449 describes more why we have a host tag. It's mostly so we can see which site is using iD so that we can track down mapping and translation issues caused by people running old versions.

In practice, people are not running multiple versions of iD in multiple iframes, and I think for usability purposes it is better to record a URL that people can actually visit.

@HolgerJeromin
Copy link
Contributor

For the current stored (many) urls IMHO we should have a solution. So i am not really sure if changing the url in the future is really needed

@bhousel
Copy link
Member Author

bhousel commented Apr 8, 2017

For the current stored (many) urls IMHO we should have a solution. So i am not really sure if changing the url in the future is really needed

Just because there is existing stored changeset tags that point to the invalid url doesn't mean we can't fix the problem. If anybody really cares, somebody could run a SQL update to change all the old values.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core An issue with one of the core iD components
Projects
None yet
Development

No branches or pull requests

2 participants