Page MenuHomePhabricator

Clicking certain references results in infinite spinner on mobile
Closed, ResolvedPublic1 Estimate Story Points

Description

The Wikipedia templates {{ref label}}, {{note label}}, {{ref}} and {{note}} don't work in mobile, producing endless loading when clicking on the footnote label. (While they have been deprecated since 2006 for citing sources, they are still used in at least 10,000 pages for note lists.) As well as not working in mobile, the usual popups on mouseover, produced by normal <ref></ref> tags, don't work for these templates.

Compare clicking "[A]" on https://en.wikipedia.org/w/index.php?title=Channel_Tunnel&oldid=757761414#Passenger_traffic_volumes with https://en.m.wikipedia.org/w/index.php?title=Channel_Tunnel&oldid=757761414#Passenger_traffic_volumes

Expected:

  • Loader should not play forever and should jump to element when none exists but there is an element that the anchor refers to in the page

Developer and reviewer notes

If there is any opportunity to make headway or update the acceptance criteria of T130551 while working on this, do it!

Details

Related Gerrit Patches:
mediawiki/extensions/MobileFrontend : masterNot-existent references should fall back to hash change

Event Timeline

Jc86035 created this task.Jan 8 2017, 4:52 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 8 2017, 4:52 AM
Jdlrobson triaged this task as Low priority.Jan 9 2017, 9:30 AM
Jdlrobson added a subscriber: Jdlrobson.

@Jc86035 can you provide an example page and tell me where I need to click to replicate the behaviour?

Jc86035 added a comment.EditedJan 9 2017, 10:23 AM

@Jdlrobson See Channel Tunnel §Passenger traffic volumes (mobile). On desktop (Vector), when moving the mouse over the blue [A] in the table header, there is no popup displaying the link. However, there is one for each of [84] and [85]. Similarly, on mobile, when [A] is clicked, the black box at the bottom displays the loading image.

I don't think this is too likely to be fixed, because the templates rely purely on CSS classes and anchors and there's already an obvious alternative for them (using normal inline <ref></ref> tags like everyone has since 2006). However, they are useful in some cases, for instance in relatively small tables where many items share the same footnotes and <references/> would produce "1. ^ a b c d e f g h i j k l m n o p q r …" before the actual note text.

Jdlrobson renamed this task from Note/note label inline reference templates are broken in mobile to Clicking certain references results in infinite spinner on mobile.Jan 9 2017, 12:40 PM
Jdlrobson raised the priority of this task from Low to Medium.
Jdlrobson updated the task description. (Show Details)

Thank you that's very useful!

No problem :)

As an aside, I wonder if this might have anything to do with adding section link/anchor support for Hovercards/Page-Previews (which is probably necessary for it to work on Wiktionary). I can't find any reported bugs though.

TheDJ added a comment.Jan 10 2017, 6:23 PM

At the very least, to take away from this issue, is the fact that very likely there is incorrect error handling in the mobile popups. It shouldn't be possible that the popups gets stuck in an infinite spinner loop :)

Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptFeb 10 2017, 7:21 PM
Jdlrobson raised the priority of this task from Medium to High.Apr 20 2017, 11:19 PM
Jdlrobson set the point value for this task to 1.
Jdlrobson updated the task description. (Show Details)Apr 27 2017, 3:49 PM
Jdlrobson updated the task description. (Show Details)

Change 352614 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] False references should fall back to hash change

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

Jdlrobson claimed this task.May 8 2017, 4:41 PM

To code reviewers: To replicate I'm directly using the example article with the IContentProvider.
The tests should give you extra confidence :)

I've left a couple of questions in the patch.

All answered.

Change 352614 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Not-existent references should fall back to hash change

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

@Jdlrobson: Are there any pages on the Beta Cluster that @ABorbaWMF could use to test this?

phuedx reassigned this task from Jdlrobson to ABorbaWMF.May 16 2017, 12:46 PM

Nope. A general QA of clicking references and checking their functionality would probably be more useful at this point given all the changes to the code. @ABorbaWMF please be sure to check in stable and beta.

@Jdlrobson, I took a look at this on beta and production. On beta, I used the Dog article, and the reference links worked fine. On production, I used the Channel Tunnel article from the original description, and the reference links worked except for the [A] reference link.

Here is what I found: if the user loads the page and taps the [A] reference link before tapping any other reference, the endless spinner appears. If the user taps another reference link first, then taps the [A] reference link, the link works just fine.

I tried on a few android and apple devices (mobile and tablet), they all displayed the same behavior. I also tested some other pages on production, and all the reference links worked well. The Channel Tunnel article is the only one I have seen with this behavior.

It doesn't work when tapped after another reference (at least from the links above); it just retains the text from the reference which was tapped before it.

The patch that fixes the bug on production will be in the next deploy. It is only fixed on the beta cluster.

Works as expected; thanks for importing the page.

Jdlrobson closed this task as Resolved.May 17 2017, 8:04 AM
Jdlrobson added a subscriber: ovasileva.

All looks good here! cc @ovasileva

Just noticed: On desktop, after the footnote has been clicked, clicking any normal reference now automatically scrolls to the fake footnote. This also seems to happen if the page URL has a section link. Does this also happen on normal mobile devices?

Jc86035 reopened this task as Open.May 30 2017, 4:49 AM
Jc86035 lowered the priority of this task from High to Low.
TheDJ added a comment.May 30 2017, 9:15 AM

@Jc86035 eh. please be more descriptive, to avoid people making incorrect interpretations of your report.. Which link, which skin, the exact links you are clicking. Are you reporting an additional issue (please file a separate ticket), or are you saying that the validation that has been done on pre-production was incorrect...

@TheDJ On the beta.wmflabs.org link above (Firefox, desktop), I clicked the [A] link, clicked the caret to go back to [A], and then clicked reference 9. The page scrolled back to [A], although the popup displayed. (I think this also happened on my phone earlier with a section link.) I'll file a separate ticket.

TheDJ added a comment.EditedMay 30 2017, 10:06 AM

I can reproduce. This is problematic. The onShowDrawer has "$window.one( 'click.drawer', $.proxy( self, 'hide' ) )" which for some reasons immediately triggers the hide() on the drawer (something like event handlers not being synchronised due to the earlier cancellation of the window I suspect....)

I also found another problem where, when you click the [ or ] of a reference you get an error, because onClickReference() assumes that you clicked the link, not the surrounding brackets

        $dest = $( ev.target ),
	href = $dest.attr( 'href' );

^^ ev.target will be text node '[ '. filed as T166548: Clicking [ or ] surrounding a reference throws an error when trying to present the reference drawer

The bug I filed is here: T166544

@Jdlrobson - has this been deployed yet? I'm still not seeing in in production (look ok on the beta cluster)

Jdlrobson closed this task as Resolved.May 30 2017, 6:34 PM

The last deployment got rolled back so this is still not fixed in production but will hopefully be this Thursday if all goes to plan. It remains fixed on https://en.m.wikipedia.beta.wmflabs.org/wiki/Channel_Tunnel

Thanks @Jc86035 @TheDJ for filing the other bugs. I will take a look.