Page MenuHomePhabricator

Incorrect marker coordinates for outer resources
Closed, ResolvedPublic

Description

Reproduction steps:

  1. Create page with mapfrme:
<mapframe text="India (80,20)" width=700 height=400 zoom=3 longitude="0" latitude="0">
{
  "type": "Feature",
  "geometry": { "type": "Point", "coordinates": [80,20] }
}
</mapframe>
  1. Open mapframe. Coordinates label will be -10.06, 28.13 (coords may be other, see also T160767). Marker will point on India. It's correct.
  2. Open Google maps link. Map will set on coords -10.06, 28.13. Marker will point on DR Congo (with the same coords). It's not correct.

Tested on test.wikipedia.org (MediaWiki 1.29.0-wmf.16 with different browsers (Firefox 50.0.2, Chrome 57.0.2987.110).

Details

Related Gerrit Patches:
mediawiki/extensions/Kartographer : wmf/1.30.0-wmf.6Reapply: Sidebar: Fix marker coordinates for outer resources
mediawiki/extensions/Kartographer : wmf/1.30.0-wmf.6Revert "Sidebar: Fix marker coordinates for outer resources"
mediawiki/extensions/Kartographer : wmf/1.30.0-wmf.5Revert "Sidebar: Fix marker coordinates for outer resources"
mediawiki/extensions/Kartographer : wmf/1.30.0-wmf.5Sidebar: Fix marker coordinates for outer resources
mediawiki/extensions/Kartographer : wmf/1.30.0-wmf.6Sidebar: Fix marker coordinates for outer resources
mediawiki/extensions/Kartographer : masterSidebar: Fix marker coordinates for outer resources

Event Timeline

IGW created this task.Mar 17 2017, 7:32 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 17 2017, 7:32 PM
IGW updated the task description. (Show Details)Mar 17 2017, 7:51 PM
  • The fullscreen mapframe may apply an offset to the initial map center since T155352: Automatically open the panel "More details" at high resolutions (otherwise the map would not look like centered correctly), As a consequence, the links on the right have that offset being applied. This is why the map appears to be centered on Congo instead of India. There are a couple of ways we can fix this:
    1. Instead of applying an offset to the map and having a hidden part of the map covered by the sidebar, resize the map container so that the sidebar doesn't overlap it anymore. The links would then open at the expected location. Note that this could create an expensive redraw every time the sidebar is toggled, which may lead to a poorer user experience.
    2. We can remove the offset applied to the map when generating the external links in the sidebar, so that these links would open at the expected location.
IGW added a comment.Mar 17 2017, 10:34 PM

My proposal is save the marker position for external services.

Let think about use case:

  1. User open fullscreen mapframe with marked some object of interest (e.g. in Wikipedia article).
  2. He explore the map (may be pan it, zoom in/out and so on) but keep in mind position of the marker.
  3. Then he open the external map. He expect that the marker and map will be in same positions. If position of the marker is change it perceives as error.

I suppose solution by pass the marker coords to the external services.
For Gmaps it will be https://maps.google.com/maps?ll={latitude},{longitude}&q={marker_latitude},{marker_longitude}&hl={language}&t=m&z={zoom} .
In my case: https://maps.google.com/maps?ll=-10.06,28.13&q=20,80&hl=en&t=m&z=3
This is identical as mapframe looks: https://test.wikipedia.org/wiki/User:IGW/sandbox#/map/0.

In both cases markers correct points to India and maps also centered on Congo.

@IGW We could do something that smart, but it would require to parse the GeoJSON for markers, which isn't necessarily the simplest thing to do.

My biggest concern is not the complexity, but rather that this solution is defined for maps that contains only 1 marker.

  • What to do if the map contains 2+ marker(s) ? 100 markers?
  • What to do if the map contains a geoshape or a geoline instead of a marker?

Basically, our solution needs to cover all the possible scenarios.

Vort added a subscriber: Vort.Mar 18 2017, 3:52 AM
IGW added a comment.Mar 18 2017, 6:13 AM

@JGirault

which isn't necessarily the simplest thing to do.

Maybe it makes sense to add marker coordinates to maplink/mapframe parameters? Also possible way pass the initial mapframe coords as the marker coord.

  • What to do if the map contains 2+ marker(s) ? 100 markers?
  • What to do if the map contains a geoshape or a geoline instead of a marker?

Can external services display all these objects? As far as I know this is no possible by URL - then don't care about it.

0x010C added a subscriber: 0x010C.May 28 2017, 1:49 PM

We have the same issue reported currently on frwiki from contributors who don't understand why the links to OSM/GMaps/... place a marker shifted from several kilometers.

Also possible way pass the initial mapframe coords as the marker coord.

Imho, this solution would fix perfectly the problem, and it seems to fit all the use-cases I've encounter (no marker, one marker, several geojson shapes, geolines/geoshapes from OSM,...).

0x010C added a comment.Jun 5 2017, 4:14 PM
This comment was removed by 0x010C.
Framawiki edited projects, added Maps; removed Maps (Kartographer).Jun 5 2017, 4:19 PM
Restricted Application added a project: Discovery. · View Herald TranscriptJun 5 2017, 4:19 PM

Change 357238 had a related patch set uploaded (by 0x010C; owner: 0x010C):
[mediawiki/extensions/Kartographer@master] Sidebar: Fix marker coordinates for outer resources

https://gerrit.wikimedia.org/r/357238

@Jdforrester-WMF Do you know who to reach to get a review for the change solving this issue? A lot of users are complaining on French Wikipedia.

@Jdforrester-WMF Do you know who to reach to get a review for the change solving this issue? A lot of users are complaining on French Wikipedia.

I don't, sorry, beyond Julien and Max.

Hi,

The Foundation Audiences and Technology Management sent a communication (also on wiki) earlier this week regarding the Discovery team's projects and goals, in response to an previous message about the re-organization of Product and Technology. There was a question regarding maps and this question and response is applicable to your ticket regarding fixing this bug. I've quoted it here:

One question: Does "As needed, assist with individual language Wikipedia’s implementation of mapframe" mean just help support the implementation on the wikis where mapframe is already deployed, or does it mean potentially deploying mapframe to additional wikis?

​We don't have the technical resources to deploy mapframe to any additional wikis at this time. If a wiki wants to deploy mapframe on their site, they can do it on their own—similar to what Swedish Wikipedia accomplished.[1]
If severe bugs occur, we'll try our best to get them fixed; such as maps aren't rendering from the tile servers. We just don't have the technical resources to do any minor bug fixes, feature requests or any new deployments right now.
[1] https://phabricator.wikimedia.org/T161032

@Yurik might be able to help if he's still doing volunteer work with maps; but I'm not sure that @MaxSem would have the time to help with this, as he's moved to a different team.

Tpt added a comment.Jun 15 2017, 6:25 PM

@debt Thank you! The fix for this bug is just changing some lines in the Kartographer extension. So, it would just require a +2 without any work for deployment

Yurik added a comment.Jun 15 2017, 9:42 PM

I might have some time to poke at it today or tomorrow. I know its hard for the WMF to do any map work after it got rid of all the map devs, so volunteers would have to step in.

Nouill added a subscriber: Nouill.Jun 23 2017, 6:07 AM

I don't very happy. Since 1 month, all the coordinate on wp:fr links to wrong location on external site (google maps, bing, with not exception) ! SO in fine, for lot and lot users :

ALL THE COORDONNATE LINKS TO WRONG LOCATION SINCE 1 MONTH FOR THOUSAND AND THOUSAND ARTICLES !

I said on wp:fr the 5 june that it will be preferable to return to geohack until the bug is remove. 0x010C answer me that the bug will be solve ... Today, 23 june, the bug is still here. I don't know myself how to return to geohack. So all I can do, it's to do caplock here....

Trizek-WMF triaged this task as High priority.Jun 23 2017, 7:19 AM
Trizek-WMF added a subscriber: Trizek-WMF.

A patch is ready and the change seems to be minor. https://gerrit.wikimedia.org/r/#/c/357238/ @Yurik ?
I'm raising up the emergency of this task because of its impact.

I'm not sure who's responsible for reviewing a patch from the community in a case like this, who has the authority to merge it, or who would be responsible for overseeing the deployment. It can't be anyone on the maps team because we have no front-end developers.

debt added a comment.EditedJun 23 2017, 2:05 PM

We're looking at the patch and have scheduled it for swat on Monday, June 26th.

debt added a comment.EditedJun 23 2017, 3:51 PM

I've been able to reproduce the issue and wanted to visually show what is going on. The map details link to google maps is using the map center point from Kartographer while ignoring the map marker on the original map. Here's our map (shown correctly):

here's how gmaps displays using the coordinates from Kartographer (using a marker on the center of the map, rather than reproducing the map marker in India):

Here is how gmaps (and any other map provider listed on the map details panel) should be showing the map from Kartographer:

Note: this bug will be presented in the next Singpost of the French community.

Change 357238 merged by jenkins-bot:
[mediawiki/extensions/Kartographer@master] Sidebar: Fix marker coordinates for outer resources

https://gerrit.wikimedia.org/r/357238

So my inability to test the patch was caused by T168744. I'll see about deploying that fix on Monday.

debt added a comment.Jun 23 2017, 8:39 PM

Thanks, @MaxSem - we're hoping to merge this patch during the eu swat window on Monday.

Change 361441 had a related patch set uploaded (by Hashar; owner: 0x010C):
[mediawiki/extensions/Kartographer@wmf/1.30.0-wmf.6] Sidebar: Fix marker coordinates for outer resources

https://gerrit.wikimedia.org/r/361441

Change 361442 had a related patch set uploaded (by Hashar; owner: 0x010C):
[mediawiki/extensions/Kartographer@wmf/1.30.0-wmf.5] Sidebar: Fix marker coordinates for outer resources

https://gerrit.wikimedia.org/r/361442

Change 361442 merged by jenkins-bot:
[mediawiki/extensions/Kartographer@wmf/1.30.0-wmf.5] Sidebar: Fix marker coordinates for outer resources

https://gerrit.wikimedia.org/r/361442

Change 361441 merged by jenkins-bot:
[mediawiki/extensions/Kartographer@wmf/1.30.0-wmf.6] Sidebar: Fix marker coordinates for outer resources

https://gerrit.wikimedia.org/r/361441

Gehel moved this task from Backlog to In progress on the Maps-Sprint board.
Gehel added a subscriber: Gehel.Jun 26 2017, 1:23 PM

When testing on mwdebug1001, we still have an issue. The google link opens to https://www.google.com/maps/place/0%C2%B000'00.0%22N+0%C2%B000'00.0%22E/@0,0,3z/data=!4m2!3m1!1s0x0:0x0?hl=fr

Here is a screenshot of the page:

Which still looks not centered correctly and with a marker at the wrong place.

Change 361447 had a related patch set uploaded (by Gehel; owner: Gehel):
[mediawiki/extensions/Kartographer@wmf/1.30.0-wmf.6] Revert "Sidebar: Fix marker coordinates for outer resources"

https://gerrit.wikimedia.org/r/361447

Change 361448 had a related patch set uploaded (by Gehel; owner: Gehel):
[mediawiki/extensions/Kartographer@wmf/1.30.0-wmf.5] Revert "Sidebar: Fix marker coordinates for outer resources"

https://gerrit.wikimedia.org/r/361448

Change 361447 merged by jenkins-bot:
[mediawiki/extensions/Kartographer@wmf/1.30.0-wmf.6] Revert "Sidebar: Fix marker coordinates for outer resources"

https://gerrit.wikimedia.org/r/361447

Change 361448 merged by jenkins-bot:
[mediawiki/extensions/Kartographer@wmf/1.30.0-wmf.5] Revert "Sidebar: Fix marker coordinates for outer resources"

https://gerrit.wikimedia.org/r/361448

@Gehel In fact, the result you got isn't an issue imho, it's the solution @IGW (the author of the issue) and I suggested earlyer in this ticket.

Also possible way pass the initial mapframe coords as the marker coord.

The mapframe in the description told to create a map centered arround the coordinates longitude="0" latitude="0", so that is what you got also on the external services like google maps.

With this patch, if you put the initial center of your map at the same location that your marker, this will work fine on external services. The code above just has to be changed in something like that to work:

<mapframe text="India (80,20)" width=700 height=400 zoom=3 longitude="80" latitude="20">
{
  "type": "Feature",
  "geometry": { "type": "Point", "coordinates": [80,20] }
}
</mapframe>
Gehel added a comment.Jun 26 2017, 7:47 PM

Ok, so this is probably a case of me not understanding what I'm doing :(

So what you are saying is that the patch is probably good, but not the test case. Which is a good news! We can try deploying it again!

Change 361584 had a related patch set uploaded (by Hashar; owner: MaxSem):
[mediawiki/extensions/Kartographer@wmf/1.30.0-wmf.6] Reapply: Sidebar: Fix marker coordinates for outer resources

https://gerrit.wikimedia.org/r/361584

Change 361584 merged by jenkins-bot:
[mediawiki/extensions/Kartographer@wmf/1.30.0-wmf.6] Reapply: Sidebar: Fix marker coordinates for outer resources

https://gerrit.wikimedia.org/r/361584

Deployed, please verify.

debt moved this task from In progress to Done on the Maps-Sprint board.Jun 26 2017, 11:56 PM

Looks good in production, thanks @MaxSem !

It seemed to work. Thank everyone, specially 0x010C .

0x010C closed this task as Resolved.Jun 27 2017, 4:48 AM
0x010C claimed this task.