Page MenuHomePhabricator

Page links inside image captions are handled as images
Closed, ResolvedPublic1 Story Points

Description

When the app is using RESTBase, some links open the gallery, even though they should just open another page.

The reason for that is that these links have the class="image" attribute set. Let's remove that since we don't need it.

Example: http://localhost:6927/en.wikipedia.org/v1/page/mobile-sections-lead/Masoretic%20text

<figcaption>The <a href=\"/wiki/Nash_Papyrus\" class=\"image\">Nash Papyrus</a>

Event Timeline

bearND created this task.Jan 13 2016, 6:34 PM
bearND raised the priority of this task from to High.
bearND updated the task description. (Show Details)
bearND moved this task to Backlog on the Mobile-Content-Service board.
bearND added a subscriber: bearND.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 13 2016, 6:34 PM
bearND claimed this task.Jan 13 2016, 6:34 PM
bearND set Security to None.

Change 263888 had a related patch set uploaded (by BearND):
Remove class=image attribute from image links

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

bearND updated the task description. (Show Details)Jan 13 2016, 6:49 PM

Change 263888 merged by jenkins-bot:
Remove class=image attribute from image links

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

Looks good to me. As an aside, the wiki page links have a visible "w:" prefix which I think is intentional.

Dbrant closed this task as Resolved.Jan 18 2016, 3:46 PM
Dbrant reopened this task as Open.May 10 2016, 2:03 AM
Dbrant added a subscriber: Dbrant.

Still seems to be happening.

Niedzielski added a comment.EditedMay 17 2016, 6:09 PM

This appears to still exist and is causing a couple crashes in the Android app. Here's one case[0]:

  1. Go to the Gases nobles article on ES Wikipedia.
  2. Scroll to the first image in the article of the spectrum.
  3. Tap the Sol link in the caption.

The gallery opens because the link class is "image" and there a couple silent crashes in the log. Attempting to share crashes the app for real.

I think there are a couple other issues[1,2] that can arise because the API assumptions become incorrect when trying to obtain image info for a non-image.

[0] https://rink.hockeyapp.net/manage/apps/226649/app_versions/17/crash_reasons/120961037?type=overview
[1] https://rink.hockeyapp.net/manage/apps/226649/app_versions/17/crash_reasons/120992191
[2] https://rink.hockeyapp.net/manage/apps/226649/app_versions/17/crash_reasons/120991565

Sounds like we should get to this one soon.

@Mholloway, I agree. It's one of our top crashes in terms of number of users affected. I'm happy to fix this but it looks like @bearND is currently on it.

MBinder_WMF set the point value for this task to 1.May 18 2016, 7:21 PM

Change 292212 had a related patch set uploaded (by Dbrant):
Fix insertion of 'image' class into figure links.

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

Change 292212 merged by jenkins-bot:
Fix insertion of 'image' class into figure links.

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

I guess we'll need to ask @mobrovac to regenerate /mobile-sections* again for all of the wikipedias for this since it's causing crashes.

Or, we can narrow it to pages where var regex = new RegExp('<figcaption.*<a.*class="image".*>.*</figcaption>', 'g'); and

lead.sections.filter(function(section) { return regex.test(section.text); }).length
          || remaining.sections.filter(function(section) { return regex.test(section.text); }).length

if that is possible/helps.

bearND added a comment.EditedJun 3 2016, 9:52 PM

We could also use a CSS selector if it's easier: Something like "figure a, span[typeof^=mw:Image] a.image" should cover it.

Dbrant closed this task as Resolved.Jun 27 2016, 4:37 PM