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

Assigned To

Authored By

	cjming
	Sep 20 2021, 7:28 PM

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

AC	Status	Details
1	✅	T291426#7393897

QA Results - Prod

AC	Status	Details
1	✅	T291426#7393902

Details

	Subject	Repo	Branch	Lines +/-
	Prevent gadgets from adding to the sticky header user menu via addPortletLink	mediawiki/skins/Vector	master	+5 -18

Customize query in gerrit

Related Objects

Mentioned In: T289816: Sticky header: Add user menu to sticky header
Mentioned Here: T302087: Sticky header dropdown has items it shouldn't have
T289816: Sticky header: Add user menu to sticky header

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.

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

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.

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

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)

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

Looks good!

@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();

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

	F34663414: Screen Recording 2021-09-30 at 5.59.09 PM.mov.gif
	Oct 1 2021, 1:00 AM

	F34663411: Screen Recording 2021-09-30 at 5.53.35 PM.mov.gif
	Oct 1 2021, 12:56 AM

Gadget-injected menu items should not be cloned to sticky header user menuClosed, ResolvedPublic3 Estimated Story PointsActions

Description

Background

Description

Acceptance criteria

QA steps

Developer notes

QA Results - Beta

QA Results - Prod

Details

Related Objects

Event Timeline

Test Result - Beta

QA Steps

Test Result - Prod

QA Steps

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