Page MenuHomePhabricator

Infobox expand click handlers running in the wrong order
Closed, ResolvedPublic2 Estimated Story Points

Description

Observed bug

Sometimes, on expanding the infobox on Special:Contributions, the data do not load. Sometimes, the user option saving whether the infobox is expanded or collapsed is incorrectly updated.

For more details, see T302285#7835425 and surrounding comments.

Reason for the bug

On clicking expand in the infobox, multiple click handlers run:

  1. From jquery.makeCollapsible, a handler runs that (among other things) toggles the mw-collapsed class
  2. From ext.ipInfo/infobox/init, a handler runs that checks the mw-collapsed class to see whether the infobox is expanded, and loads or does not load the data, accordingly
  3. A second handler from ext.ipInfo/infobox/init also checks the mw-collapsed class and updates the user option (or localStorage) accordingly

If (2) and/or (3) run before (1), they will make the wrong inference about whether the infobox is expanded.

Acceptance criteria
  • The expand/collapse functionality of the infobox works as intended
Testing notes

I could reproduce the bug consistently by setting $wgShowDebug = true; in LocalSettings.php

Event Timeline

AGueyte set the point value for this task to 2.Apr 11 2022, 3:19 PM

jquery.makeCollapsible.js and infobox/init.js both add the event handlers (from the task description) using jQuery. That means they will always run in the order added.

The problem is that jquery.makeCollapsible.js isn't guaranteed to run before infobox/init.js - if it runs after, then the handlers will be added in the wrong order. (For testing this: I've found that locally they get added in the wrong order if I enable debugging, but in the right order if I disable it.)

As discussed in gerrit, on the patch for T302285: Infobox not always loading correctly on Special:Contributions; possible race-condition:

[...] makeCollapsible is running from this script [...]: https://gerrit.wikimedia.org/g/mediawiki/core/+/87d9978e8949dc1564aba6dba6b7b893317a78de/resources/src/mediawiki.page.ready/ready.js#14

There doesn't seem to be a simple way to ensure that makeCollapsible is called from here before infobox/init.js runs, because it is wrapped in an async call:
mw.loader.using( dependencies ).then( ... )

[...] The only reason we needed makeCollapsible to run first, was so we could check the aria-expanded attribute, which makeCollapsible adds. However, we can check the collapsed state from the presence/absence of the mw-collapsed class, which is added before makeCollapsible runs. [...]

It turns out that the last part of that comment is incorrect - we also need makeCollapsible to run first so that the event handlers get added in the correct order.

Solution: we need to manually run jquery.makeCollapsible from the init script.

(A more ideal solution would be to have a component that models its expanded/collapsed state, but here we're working with what's already available.)

Change 779081 had a related patch set uploaded (by Tchanders; author: Tchanders):

[mediawiki/extensions/IPInfo@master] Ensure event handlers run in the correct order for the infobox

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

Change 779081 merged by jenkins-bot:

[mediawiki/extensions/IPInfo@master] Ensure event handlers run in the correct order for the infobox

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

dom_walden subscribed.

I have not been able to reproduce either the problem of:

  • the infobox not expanding or collapsing
  • the wrong state of the infobox expand/collapse being saved.

I mostly used Firefox to try to reproduce, as this was the browser I had the most trouble with previously. I also tried to reproduce by automating the browser, so I could repeat the reproduction steps in a loop 50-100 times.

I also tried to reproduce them on Chromium and Safari (14 and 15), but with less conviction.

There was a weird issue with Safari which I could reproduce on Saucelabs. Sometimes the toggle button and "IP Information" title does not appear when the page loads (see screenshots). But none of the other members of the team who have tried so far have been able to reproduce. I won't investigate this further, unless we get some evidence this isn't an issue specific to Saucelabs.

safari_14_infobox.png (860×1 px, 385 KB)

safari_15_infobox.png (925×1 px, 472 KB)

Test environment: https://en.wikipedia.beta.wmflabs.org IP Info 0.0.0 (3ff1b29) 19:53, 13 April 2022.