Page MenuHomePhabricator

Inconsistent parameter passed to the wikipage.indicators JS hook
Open, Needs TriagePublicBUG REPORT

Description

Steps to reproduce

  1. Enable Show previews without reloading the page in Preferences → Editing if you haven’t done already.
  2. Navigate to a page with page indicator(s), e.g. https://en.wikipedia.org/wiki/Talk:Main_Page
  3. Open the browser console.
  4. Enter void mw.hook( 'wikipage.indicators' ).add( console.log ); and observe the logged jQuery object.
  5. Now open the page for editing, and preview it.
  6. Enter void mw.hook( 'wikipage.indicators' ).add( console.log ); again and observe the logged jQuery object.

Actual result

The two logged jQuery objects are different: the first one contains the <div class="mw-indicators">, the second one contains a <div class="mw-indicator"> (plus a text node consisting of a single newline).

Expected result

The two logged jQuery objects are the same. I don’t really mind which one, but according to the documentation, it should be the <div class="mw-indicators">, not its <div class="mw-indicator"> child(ren).

Additional information

The code was added in bea70565 by @Fomafix.

Event Timeline

This is inconsistent and was not intended.

In mediawiki.action.edit.preview the hook gets fired before adding to the DOM. Therefor it can't fired with the root element here. So be consisted the hook in mediawiki.page.ready should fired with children elements.

Change 731262 had a related patch set uploaded (by Fomafix; author: Fomafix):

[mediawiki/core@master] mediawiki.page.ready: Fire hook 'wikipage.indicators' with childrens

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

Change 732071 had a related patch set uploaded (by Krinkle; author: Fomafix):

[mediawiki/core@REL1_37] mediawiki.page.ready: Fire hook 'wikipage.indicators' with children

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

@Fomafix Can you prepare a cherry pick for REL1_36 as well? (There is a merge conflict as-is.)

Change 732071 merged by jenkins-bot:

[mediawiki/core@REL1_37] mediawiki.page.ready: Fire hook 'wikipage.indicators' with children

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

Change 731262 merged by jenkins-bot:

[mediawiki/core@master] mediawiki.page.ready: Fire hook 'wikipage.indicators' with children

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

Change 732075 had a related patch set uploaded (by Fomafix; author: Fomafix):

[mediawiki/core@REL1_36] mediawiki.page.ready: Fire hook 'wikipage.indicators' with children

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

@Fomafix Can you prepare a cherry pick for REL1_36 as well? (There is a merge conflict as-is.)

Done in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/732075

Change 732075 merged by jenkins-bot:

[mediawiki/core@REL1_36] mediawiki.page.ready: Fire hook 'wikipage.indicators' with children

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