Page MenuHomePhabricator
Create Task
Maniphest T293564

Inconsistent parameter passed to the wikipage.indicators JS hook
Closed, ResolvedPublicBUG REPORT
Actions

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.

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
mediawiki.page.ready: Fire hook 'wikipage.indicators' with childrenmediawiki/coreREL1_36+24 -19
mediawiki.page.ready: Fire hook 'wikipage.indicators' with childrenmediawiki/coremaster+24 -19
mediawiki.page.ready: Fire hook 'wikipage.indicators' with childrenmediawiki/coreREL1_37+24 -19
Customize query in gerrit

Related Objects

Event Timeline

Tacsipacsi created this task.Oct 17 2021, 12:03 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 17 2021, 12:03 AM
Fomafix added a subscriber: Krinkle.EditedOct 17 2021, 6:51 AM

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.

Fomafix claimed this task.Oct 17 2021, 7:12 AM
gerritbot added a comment.Oct 17 2021, 7:12 AM

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

gerritbot added a project: Patch-For-Review.Oct 17 2021, 7:12 AM
gerritbot added a comment.Oct 20 2021, 12:25 AM

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

Krinkle added a comment.Oct 20 2021, 12:26 AM

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

gerritbot added a comment.Oct 20 2021, 12:35 AM

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

gerritbot added a comment.Oct 20 2021, 12:45 AM

Change 731262 merged by jenkins-bot:

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

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

ReleaseTaggerBot added projects: MW-1.37-notes, MW-1.38-notes (1.38.0-wmf.6; 2021-10-26).Oct 20 2021, 1:00 AM
Maintenance_bot removed a project: Patch-For-Review.Oct 20 2021, 1:10 AM
gerritbot added a comment.Oct 20 2021, 8:01 AM

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

gerritbot added a project: Patch-For-Review.Oct 20 2021, 8:01 AM
Fomafix added a comment.Oct 20 2021, 8:09 AM
In T293564#7443057, @Krinkle wrote:

@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

gerritbot added a comment.Oct 20 2021, 2:00 PM

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

Maintenance_bot removed a project: Patch-For-Review.Oct 20 2021, 2:10 PM
ReleaseTaggerBot added a project: MW-1.36-notes.Oct 20 2021, 3:00 PM
Aklapper removed Fomafix as the assignee of this task.May 16 2024, 4:38 PM

@Fomafix: Removing task assignee as this open task has been assigned for more than two years - see the email sent to all task assignees on 2024-04-15.
Please assign this task to yourself again if you still realistically [plan to] work on this task - it would be welcome! :)
If this task has been resolved in the meantime, or should not be worked on by anybody ("declined"), please update its task status via "Add Action… 🡒 Change Status".
Also see https://www.mediawiki.org/wiki/Bug_management/Assignee_cleanup for tips how to best manage your individual work in Phabricator. Thanks!

Tacsipacsi closed this task as Resolved.May 18 2024, 10:08 AM
Tacsipacsi assigned this task to Fomafix.

I believe this has already been fixed in 8bec93a573f9a6a92a721a9242db8dffb88f405e by @Fomafix.

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits