Page MenuHomePhabricator

Regression: Nested references do not open if user clicks on [ or ] (which are wrapped in span)
Closed, ResolvedPublic

Description

Related: T243822

Visit https://en.m.wikipedia.beta.wmflabs.org/wiki/Nested_references
Click [note 1]
click [1]
Repeat.

Expected:
The drawer updates

Actual:
It works the first time, but not the second time.


Developer notes:

The underlying HTML has changed exposing a flaw in the click handler - this means clicking the [ or ] doesn't open the nested reference:

Before:

<sup id="cite_ref-2" class="reference"><a href="#cite_note-2">[note 1]</a></sup>

Now:

<sup id="cite_ref-2" class="reference"><a href="#cite_note-2"><span>[</span>note 1<span>]</span></a></sup>

Acceptance criteria

  • test is skipped
  • MobileFrontend is patched
  • Test is no longer skipped in Minerva

QA Results - Beta

ACStatusDetails
1T258096#6434480

QA Results - Prod

ACStatusDetails
1T258096#6434894

QA Results - Prod (Sept 10, confirmed 1.36.0-wmf.8)

ACStatusDetails
1T258096#6452585

Event Timeline

Restricted Application added subscribers: Masumrezarock100, Aklapper. · View Herald TranscriptJul 15 2020, 7:12 PM
Jdlrobson added a project: Readers-Web-Backlog.
Jdlrobson moved this task from Incoming to Needs Prioritization on the Readers-Web-Backlog board.
Jdlrobson triaged this task as High priority.Jul 21 2020, 8:05 PM

This is a recent regression and is causing various browser test failures.

Jdlrobson updated the task description. (Show Details)

I believe this relates to Reference Previews - both Popups previews and MobileFrontend are conflicting with one another - only one of those should be applied. We might need to sink with WMDE and resolve T243822 to handle this one.

Change 615298 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Nested references should not trigger console warning

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

Jdlrobson renamed this task from Nested references do not always open on MInerva to Browser test failure: Nested references do not always open.Jul 23 2020, 7:49 PM

Change 615298 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Nested references should not trigger console warning

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

Jdlrobson reassigned this task from ovasileva to Edtadros.Jul 28 2020, 6:42 PM
Jdlrobson added a subscriber: ovasileva.

Change 616891 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Fix faulty nested references click handler

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

Jdlrobson renamed this task from Browser test failure: Nested references do not always open to Regression: Nested references do not open if user clicks on [ or ] (which are wrapped in span).Jul 28 2020, 7:06 PM
Jdlrobson updated the task description. (Show Details)

Change 616899 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] QA: References clicks should apply to the sup element not span

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

Jdlrobson lowered the priority of this task from High to Medium.Jul 28 2020, 10:02 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson added a project: Regression.

Thanks to @Edtadros we've skipped the test, so the browser test build is green again, but there is a real regression here we should fix caused by some underlying parser changes in I2cc88069b19e7611f23c83ca993f9caa70f786f0

I'd be tempted to just skip this and move on as it looks like it will be tricky to debug but on the other hand it is a useful test for something we seldom check.

Jdlrobson updated the task description. (Show Details)
Jdlrobson added a subscriber: Edtadros.
Ammarpad added a subscriber: Ammarpad.EditedMon, Aug 31, 7:46 AM

Hi @Jdlrobson, I tested this and the patch but unfortunately could not reproduce the issue locally, and also could not on English Wikipedia. I also tried on real Mobile device, just in case. All the tests are on Chrome desktop/mobile Browser 84.

Can you try and see if you can reproduce on English Wikipedia https://en.m.wikipedia.org/wiki/User:Ammarpad/sandbox?

On Betawiki, I can reproduce it, and I noted that the HTMLs generated by my local wiki and English Wikipedia match, and subsequently matches the HTML you gave above as "before"

<sup id="cite_ref-2" class="reference"><a href="#cite_note-2">[note 1]</a></sup>

In other words, the HTML generated where it works is different from the one generated on Betawiki (https://en.m.wikipedia.beta.wmflabs.org/wiki/Nested_references) where it does not. It sounds to me like something is interfering with the generated HTML on Beta cluster but not in production. I am not sure what's that yet. (I just want make sure that it's not only me who cannot reproduce on English Wikipedia before checking further what might cause this)

My guess is this relates to Parsoid since the HTML output is different. Maybe someone from Parsing-Team ( @cscott ?) can explain the HTML change to references we are seeing on beta cluster?

Jdrewniak claimed this task.Wed, Sep 2, 5:11 PM

Per Jdlrobson's findings, this may have to do with the local modification to https://en.wikipedia.beta.wmflabs.org/w/index.php?title=MediaWiki:Cite_reference_link&action=history.

default = <sup id="$1" class="reference">[[#$2|&#91;$3&#93;]]</sup>
modified = <sup id="$1" class="reference">[[#$2|<span><nowiki>[</nowiki></span>$3<span><nowiki>]</nowiki></span>]]</sup>

The on wiki change is, however, not new as it's over a decade old, so probably there has to be something that actually triggered the test failures/regression since they were working before then.

Change 616891 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Harden nested references click handler against different markup

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

Jdlrobson reassigned this task from Jdrewniak to Edtadros.Wed, Sep 2, 10:54 PM
Jdlrobson added a subscriber: Jdrewniak.

Change 616899 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] QA: Restore nested reference test

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

Mhurd claimed this task.Thu, Sep 3, 7:35 PM
Mhurd added a comment.Thu, Sep 3, 8:24 PM

Test Result - Beta

Status: ✅ PASS
Environment: beta
OS: macOS Catalina
Browser: Chrome
Device: MBP
Emulated Device: iPhone X
Test Artifact(s):

QA steps

Visit https://en.m.wikipedia.beta.wmflabs.org/wiki/Nested_references
Click [note 1]
click [1]
Repeat.

Mhurd added a comment.EditedThu, Sep 3, 11:58 PM

Test Result - Prod

Status: ✅ PASS
Environment: enwiki
OS: macOS Catalina
Browser: Chrome
Device: MBP
Emulated Device: iPhone X
Test Artifact(s):

QA steps

Visit https://en.m.wikipedia.org/wiki/User:Montehurd/Nested_references
Click [note 1]
click [1]
Repeat.

Looks like prod is on 1.36.0-wmf.6, and even though ReleaseTaggerBot says 1.36.0-wmf.8 it seems to work fine. In the recording attached to this comment I'm not focusing on clicking precisely on the brackets, but I did so on both Safari and Chrome after recording and everything still seemed to work fine... 🤔

Mhurd updated the task description. (Show Details)Thu, Sep 3, 11:59 PM
Mhurd added a comment.Fri, Sep 4, 12:16 AM

@Jdlrobson Hope I didn't goof things here... I was going on the description when I recorded and only noticed the ticket title mentions the brackets themselves after I had recorded... 🤞

@Mhurd per T258096#6430908 this related to a bug in beta cluster which was thankfully never live on production. We may want to verify however that these changes don't break production in any way when it goes live next Thursday (although that would be very very surprising!).

Test Result - Prod (confirmed 1.36.0-wmf.8)

Status: ✅ PASS
Environment: enwiki
OS: macOS Catalina
Browser: Chrome
Device: MBP
Emulated Device: iPhone X
Test Artifact(s):

QA steps

Visit https://en.m.wikipedia.org/wiki/User:Montehurd/Nested_references
Click [note 1]
click [1]
Repeat.

Mhurd reassigned this task from Mhurd to ovasileva.Thu, Sep 10, 10:25 PM
Mhurd updated the task description. (Show Details)
Mhurd added a subscriber: Mhurd.
ovasileva closed this task as Resolved.Fri, Sep 11, 8:56 AM