Page MenuHomePhabricator
Create Task
Maniphest T291426

Gadget-injected menu items should not be cloned to sticky header user menu
Closed, ResolvedPublic3 Estimated Story Points
Actions

Description

Background

As part of the AC for T289816, gadget-injected menu items into the personal menu are cloned to the user menu of the new sticky header feature that is currently in development.

This comment on the patch associated with the aforementioned ticket outlines an issue encountered with a specific gadget called the UTC live clock which is no longer maintained. Because of how the javascript for this gadget is written, live updating of the clock relies on the original id of the cloned node (we change the ids of all cloned items to be unique in the sticky header user menu). Unless we refactor the gadget code in this particular use case, the gadget-injected menu item does not work as expected in the sticky header user menu.

Until or if we offer configuration options for gadgets to include menu links in the personal menu, cloning these items does not guarantee that they will function properly. To avoid unexpected breaking of menu items, we should remove gadget-injected menu items from the sticky header user menu for the time being.

Description

Remove gadget-injected items from the user menu of the sticky header.

Acceptance criteria

  • Any gadgets that load late should not add items to the user menu of the sticky header via mw.hook( 'util.addPortletLink' ).

QA steps

  • Run the following in Javascript console:
setTimeout( function () {
mw.util.addPortletLink('p-personal', 'Test', 'Test')
}, 1000  )
  • Test should appear in the user menu of the main/fixed header, but not in the sticky header user menu.

Developer notes

This is blocked on https://gerrit.wikimedia.org/r/c/mediawiki/core/+/720112 which adds a class allowing us to filter items added via JavaScript.

QA Results - Beta

ACStatusDetails
1✅T291426#7393897

QA Results - Prod

ACStatusDetails
1✅T291426#7393902

Details

SubjectRepoBranchLines +/-
Prevent gadgets from adding to the sticky header user menu via addPortletLinkmediawiki/skins/Vectormaster+5 -18
Customize query in gerrit

Related Objects

Event Timeline

cjming created this task.Sep 20 2021, 7:28 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 20 2021, 7:28 PM
• nray subscribed.Sep 20 2021, 7:32 PM
ovasileva triaged this task as High priority.Sep 20 2021, 7:35 PM
Jdlrobson moved this task from Needs Analysis to Upcoming on the Web-Team-Backlog (Kanbanana-FY-2021-22) board.Sep 21 2021, 5:24 PM
Jdlrobson updated the task description. (Show Details)
LGoto moved this task from Upcoming to Ready for Development on the Web-Team-Backlog (Kanbanana-FY-2021-22) board.Sep 21 2021, 5:39 PM
LGoto set the point value for this task to 3.
bwang subscribed.EditedSep 22 2021, 3:53 PM

Don't we just have to delete this code in order for the AC here?
https://github.com/wikimedia/Vector/blob/master/resources/skins.vector.js/stickyHeader.js#L115-L131

Not sure if this actually needs the mw-list-item-js class added by https://gerrit.wikimedia.org/r/c/mediawiki/core/+/720112

gerritbot added a comment.Sep 22 2021, 5:34 PM

Change 722938 had a related patch set uploaded (by Bernard Wang; author: Bernard Wang):

[mediawiki/skins/Vector@master] Prevent gadgets from adding to the sticky header user menu via addPortletLink

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

gerritbot added a project: Patch-For-Review.Sep 22 2021, 5:34 PM
bwang claimed this task.Sep 22 2021, 6:46 PM
bwang removed bwang as the assignee of this task.
bwang moved this task from Ready for Development to Doing on the Web-Team-Backlog (Kanbanana-FY-2021-22) board.
bwang moved this task from Doing to Code Review on the Web-Team-Backlog (Kanbanana-FY-2021-22) board.
bwang moved this task from Code Review to Needs More Work on the Web-Team-Backlog (Kanbanana-FY-2021-22) board.Sep 23 2021, 4:59 PM
Jdlrobson claimed this task.Sep 23 2021, 5:13 PM
Jdlrobson moved this task from Needs More Work to Code Review on the Web-Team-Backlog (Kanbanana-FY-2021-22) board.
LGoto reassigned this task from Jdlrobson to bwang.Sep 23 2021, 5:13 PM
LGoto added a subscriber: Jdlrobson.
Jdlrobson moved this task from Code Review to Doing on the Web-Team-Backlog (Kanbanana-FY-2021-22) board.Sep 23 2021, 5:13 PM
bwang removed bwang as the assignee of this task.Sep 23 2021, 8:46 PM
bwang moved this task from Doing to Code Review on the Web-Team-Backlog (Kanbanana-FY-2021-22) board.
gerritbot added a comment.Sep 23 2021, 9:46 PM

Change 722938 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Prevent gadgets from adding to the sticky header user menu via addPortletLink

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

ReleaseTaggerBot added a project: MW-1.38-notes (1.38.0-wmf.2; 2021-09-28).Sep 23 2021, 10:00 PM
Maintenance_bot removed a project: Patch-For-Review.Sep 23 2021, 10:10 PM
Jdlrobson assigned this task to Edtadros.Sep 23 2021, 10:25 PM
Jdlrobson moved this task from Code Review to QA on the Web-Team-Backlog (Kanbanana-FY-2021-22) board.
cjming mentioned this in T289816: Sticky header: Add user menu to sticky header.Sep 29 2021, 4:01 PM
Edtadros moved this task from QA to QA in Prod on the Web-Team-Backlog (Kanbanana-FY-2021-22) board.Oct 1 2021, 12:56 AM

Test Result - Beta

Status: ✅ PASS
Environment: beta
OS: macOS Big Sur
Browser: Chrome
Device: MBP
Emulated Device: NA

Test Artifact(s):

QA Steps

Run the following in Javascript console:

setTimeout( function () {
mw.util.addPortletLink('p-personal', 'Test', 'Test')
}, 1000  )

✅ AC1: Test should appear in the user menu of the main/fixed header, but not in the sticky header user menu.

Screen Recording 2021-09-30 at 5.53.35 PM.mov.gif (662×1 px, 1 MB)

Edtadros reassigned this task from Edtadros to ovasileva.Oct 1 2021, 1:00 AM
Edtadros moved this task from QA in Prod to Ready for Signoff on the Web-Team-Backlog (Kanbanana-FY-2021-22) board.
Edtadros subscribed.

Test Result - Prod

Status: ✅ PASS
Environment: enwiki
OS: macOS Big Sur
Browser: Chrome
Device: MBP
Emulated Device: NA

Test Artifact(s):

QA Steps

Run the following in Javascript console:

setTimeout( function () {
mw.util.addPortletLink('p-personal', 'Test', 'Test')
}, 1000  )

✅ AC1: Test should appear in the user menu of the main/fixed header, but not in the sticky header user menu.

Screen Recording 2021-09-30 at 5.59.09 PM.mov.gif (662×1 px, 678 KB)

Edtadros updated the task description. (Show Details)Oct 1 2021, 1:04 AM
ovasileva closed this task as Resolved.Oct 5 2021, 4:48 PM

Looks good!

Od1n subscribed.Feb 18 2022, 12:52 PM

@Jdlrobson The below code (found here) seems to be incorrect:

// Remove portlet links added by gadgets using mw.util.addPortletLink, T291426
const gadgetLinks = userMenuClone.querySelector( 'mw-list-item-js' );
if ( gadgetLinks ) {
	gadgetLinks.remove();
}
  • misses the leading dot of class selector
  • querySelector() returns only the first matching element, but here we may have several matches

Correct code would be (using jQuery):

// Remove portlet links added by gadgets using mw.util.addPortletLink, T291426
$( userMenuClone ).find( '.mw-list-item-js' ).remove();
Jdlrobson added a comment.Feb 18 2022, 4:49 PM

Thanks for the report @Od1n I've created T302087 to fix that.

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