popupNotification Fix memory leaks
Closed, ResolvedPublic2 Estimated Story PointsBUG REPORT
Actions

Assigned To

None

Authored By

	• Mabualruz
	Apr 20 2023, 4:50 PM

Description

We believe popupNotifications.js has a memory leak because we create new popup widgets each time we show the popup, and since the OOUI popup widget doesn't have a destructor, we just remove the popup from the DOM.
We believe the simplest solution is to store an array of all popup Widgets, so that we don't reinitialize new widgets each time a popup notification is created

QA

Repeat QA we did on T334366

QA Results - Beta

AC	Status	Details
1	✅	T335153#8817510
2	✅	T335153#8817510
3	✅	T335153#8828699
4	✅	T335153#8817510
5	✅	T335153#8817510

QA Results - Prod

AC	Status	Details
1	✅	T335153#8848519
2	✅	T335153#8848519
3	✅	T335153#8848519
4	✅	T335153#8848519
5	✅	T335153#8848519

Details

Other Assignee: • Mabualruz

Related Changes in Gerrit:

Subject	Repo	Branch	Lines +/-
Ensure page load popupNotification is closed when the toggle button is clicked	mediawiki/skins/Vector	wmf/1.41.0-wmf.7	+7 -3
Ensure page load popupNotification is closed when the toggle button is clicked	mediawiki/skins/Vector	master	+7 -3
popupNotification Fix memory leaks	mediawiki/skins/Vector	master	+2 -1
popupNotification Fix memory leaks	mediawiki/skins/Vector	master	+53 -42

Customize query in gerrit

Related Objects

Mentioned Here: T334366: Make it possible for indicator to show on page load

Event Timeline

• Mabualruz created this task.Apr 20 2023, 4:50 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 20 2023, 4:50 PM

• Mabualruz updated the task description. (Show Details)Apr 20 2023, 4:51 PM

• Mabualruz updated Other Assignee, added: • Mabualruz.Apr 20 2023, 4:58 PM

• Mabualruz added a subscriber: Jdlrobson.

Change 910081 had a related patch set uploaded (by Mabualruz; author: Mabualruz):

[mediawiki/skins/Vector@master] popupNotification Fix memory leaks

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

gerritbot added a project: Patch-For-Review.Apr 20 2023, 5:05 PM

@ovasileva this is likely a blocker for deploying the pointer overlay.

ovasileva triaged this task as High priority.Apr 20 2023, 9:11 PM

• Mabualruz moved this task from Incoming to Code Review on the Web-Team-Backlog-Archived (Web Team FY2022-23 Q4 Sprint 2) board.Apr 24 2023, 1:14 PM

• Mabualruz claimed this task.Apr 24 2023, 5:17 PM

LGoto set the point value for this task to 2.Apr 24 2023, 5:35 PM

Jdlrobson moved this task from Code Review to Needs More Work on the Web-Team-Backlog-Archived (Web Team FY2022-23 Q4 Sprint 2) board.Apr 25 2023, 12:57 AM

• Mabualruz moved this task from Needs More Work to Code Review on the Web-Team-Backlog-Archived (Web Team FY2022-23 Q4 Sprint 2) board.Apr 25 2023, 2:29 PM

I noticed an odd behaviour when scrolling down the page tools do not move to the sticky header while everything else from the menu does.

@ovasileva if that is intended, please let me know. else I will open a task to have the page tools on the sticky header.

In T335153#8804818, @Mabualruz wrote:

I noticed an odd behaviour when scrolling down the page tools do not move to the sticky header while everything else from the menu does.

@ovasileva if that is intended, please let me know. else I will open a task to have the page tools on the sticky header.

Yes, page tools are not available in the sticky header

Jdlrobson moved this task from Code Review to QA on the Web-Team-Backlog-Archived (Web Team FY2022-23 Q4 Sprint 2) board.Apr 27 2023, 6:28 PM

Change 910081 merged by jenkins-bot:

[mediawiki/skins/Vector@master] popupNotification Fix memory leaks

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

Maintenance_bot removed a project: Patch-For-Review.Apr 27 2023, 7:10 PM

When the patch merges it will be possible to test this in an incognito window on https://en.wikipedia.beta.wmflabs.org/

@Mabualruz I'm seeing multiple popup elements in the dom when i repeatedly pin/unpin the TOC https://jmp.sh/l2UqeCT6

bwang reassigned this task from Edtadros to • Mabualruz.Apr 28 2023, 5:55 PM

bwang moved this task from QA to Needs More Work on the Web-Team-Backlog-Archived (Web Team FY2022-23 Q4 Sprint 2) board.

bwang added a subscriber: Edtadros.

Change 912405 had a related patch set uploaded (by Mabualruz; author: Mabualruz):

[mediawiki/skins/Vector@master] popupNotification Fix memory leaks

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

gerritbot added a project: Patch-For-Review.May 1 2023, 10:12 AM

• Mabualruz reassigned this task from • Mabualruz to bwang.May 1 2023, 10:47 AM

• Mabualruz moved this task from Needs More Work to Code Review on the Web-Team-Backlog-Archived (Web Team FY2022-23 Q4 Sprint 2) board.

bwang removed bwang as the assignee of this task.May 1 2023, 4:21 PM

bwang moved this task from Code Review to QA on the Web-Team-Backlog-Archived (Web Team FY2022-23 Q4 Sprint 2) board.

Change 912405 merged by jenkins-bot:

[mediawiki/skins/Vector@master] popupNotification Fix memory leaks

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

Maintenance_bot removed a project: Patch-For-Review.May 1 2023, 5:10 PM

Jdlrobson assigned this task to Edtadros.May 1 2023, 5:34 PM

Jdlrobson updated the task description. (Show Details)

Test Result - Beta

Status: ❌
Environment: beta
OS: macOS Ventura
Browser: Chrome
Device: MBP
Emulated Device:NA

Test Artifact(s):

QA Steps

✅ AC1: On page load for users who have not interacted with the toggle
Set up a pointer overlay that highlights the page width toggle.
Tooltip copy: "You can toggle between a limited width and full width by clicking this button."
Tooltip should show for all logged-out users on page load until dismissed via the "Dismiss" button.

Screenshot 2023-05-01 at 1.54.49 PM.png (1×1 px, 532 KB)

✅ AC2: On page load for users who have previously interacted with the toggle to enable full width without realizing what it did
Tooltip copy: "You have switched your layout to full width. To go back to limited width, press this button."
Tooltip should show for all logged-out users on page load until dismissed via the "Dismiss" button.

Screenshot 2023-05-01 at 1.55.15 PM.png (1×1 px, 544 KB)

❌ AC3: Clicking the button
Clicking the limited width button will display a secondary tooltip that informs you how to go back to fixed width
Tooltip copy: "You have switched your layout to full width. To go back to limited width, press this button."
The secondary tooltip should not show if you restore limited width, but should show if you click the button to switch to full width again.
When clicking it again (to restore limited width), the tooltip should fade out.
@Jdlrobson, the tool tips do not fade out.

Screen Recording 2023-05-01 at 1.55.53 PM.mov.gif (1×1 px, 1 MB)

✅ AC4: With cookies disabled
on page load I do not see a popup
on disabling limited width I see the popup

Screen Recording 2023-05-01 at 2.00.34 PM.mov.gif (1×1 px, 748 KB)

Jdlrobson reassigned this task from Jdlrobson to • Mabualruz.May 1 2023, 10:03 PM

Change 913942 had a related patch set uploaded (by Mabualruz; author: Mabualruz):

[mediawiki/skins/Vector@master] popupNotification Fix memory leaks

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

gerritbot added a project: Patch-For-Review.May 2 2023, 9:45 PM

@Edtadros the 1st ever popup has to be dismissed so we attach the cookie that the user have been informed of the feature, then it would be the fade behaviour.

• Mabualruz reassigned this task from • Mabualruz to bwang.May 2 2023, 9:47 PM

• Mabualruz moved this task from Needs More Work to Code Review on the Web-Team-Backlog-Archived (Web Team FY2022-23 Q4 Sprint 2) board.

bwang reassigned this task from bwang to • Mabualruz.May 3 2023, 7:24 PM

bwang moved this task from Code Review to Needs More Work on the Web-Team-Backlog-Archived (Web Team FY2022-23 Q4 Sprint 2) board.

Jdlrobson reassigned this task from • Mabualruz to Edtadros.May 3 2023, 9:32 PM

Jdlrobson moved this task from Needs More Work to QA on the Web-Team-Backlog-Archived (Web Team FY2022-23 Q4 Sprint 2) board.

Change 913942 merged by jenkins-bot:

[mediawiki/skins/Vector@master] Ensure page load popupNotification is closed when the toggle button is clicked

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

Maintenance_bot removed a project: Patch-For-Review.May 3 2023, 10:10 PM

Test Result - Beta

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

Test Artifact(s):

QA Steps

✅ AC3: Clicking the button
Clicking the limited width button will display a secondary tooltip that informs you how to go back to fixed width
Tooltip copy: "You have switched your layout to full width. To go back to limited width, press this button."
The secondary tooltip should not show if you restore limited width, but should show if you click the button to switch to full width again.
When clicking it again (to restore limited width), the tooltip should fade out.

Screen Recording 2023-05-04 at 8.16.40 PM.mov.gif (1×2 px, 870 KB)

Edtadros updated the task description. (Show Details)May 5 2023, 3:29 AM

Change 917162 had a related patch set uploaded (by Jdlrobson; author: Mabualruz):

[mediawiki/skins/Vector@wmf/1.41.0-wmf.7] Ensure page load popupNotification is closed when the toggle button is clicked

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

gerritbot added a project: Patch-For-Review.May 8 2023, 6:36 PM

Change 917162 merged by jenkins-bot:

[mediawiki/skins/Vector@wmf/1.41.0-wmf.7] Ensure page load popupNotification is closed when the toggle button is clicked

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

Mentioned in SAL (#wikimedia-operations) [2023-05-08T20:21:55Z] <taavi@deploy1002> Started scap: Backport for [[gerrit:917162|Ensure page load popupNotification is closed when the toggle button is clicked (T335153)]]

Mentioned in SAL (#wikimedia-operations) [2023-05-08T20:23:11Z] <taavi@deploy1002> jdlrobson and taavi: Backport for [[gerrit:917162|Ensure page load popupNotification is closed when the toggle button is clicked (T335153)]] synced to the testservers: mwdebug1001.eqiad.wmnet, mwdebug1002.eqiad.wmnet, mwdebug2001.codfw.wmnet, mwdebug2002.codfw.wmnet

Mentioned in SAL (#wikimedia-operations) [2023-05-08T20:29:53Z] <taavi@deploy1002> Finished scap: Backport for [[gerrit:917162|Ensure page load popupNotification is closed when the toggle button is clicked (T335153)]] (duration: 07m 58s)

Maintenance_bot removed a project: Patch-For-Review.May 8 2023, 8:30 PM

ovasileva moved this task from Ready for Signoff to QA in Production on the Web-Team-Backlog-Archived (Web Team FY2022-23 Q4 Sprint 2) board.May 11 2023, 6:37 PM

Test Result - Prod

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

Test Artifact(s):

QA Steps

Screenshot 2023-05-12 at 1.58.04 PM.png (738×1 px, 358 KB)

Screenshot 2023-05-12 at 1.57.39 PM.png (702×1 px, 345 KB)

Screen Recording 2023-05-12 at 1.59.40 PM.mov.gif (736×1 px, 1 MB)

✅ AC4: With cookies disabled
on page load I do not see a popup
on disabling limited width I see the popup

Screen Recording 2023-05-12 at 2.02.54 PM.mov.gif (736×1 px, 1 MB)

Edtadros updated the task description. (Show Details)May 12 2023, 9:05 PM

ReleaseTaggerBot added a project: MW-1.41-notes (1.41.0-wmf.9; 2023-05-15).May 16 2023, 12:02 AM

ReleaseTaggerBot edited projects, added MW-1.41-notes (1.41.0-wmf.10; 2023-05-23); removed MW-1.41-notes (1.41.0-wmf.9; 2023-05-15).May 16 2023, 4:01 AM

ReleaseTaggerBot edited projects, added MW-1.41-notes (1.41.0-wmf.7; 2023-05-01); removed MW-1.41-notes (1.41.0-wmf.10; 2023-05-23).May 16 2023, 3:02 PM

	F36994630: Screen Recording 2023-05-12 at 2.02.54 PM.mov.gif
	May 12 2023, 9:04 PM

	F36994627: Screen Recording 2023-05-12 at 1.59.40 PM.mov.gif
	May 12 2023, 9:04 PM

	F36994624: Screenshot 2023-05-12 at 1.57.39 PM.png
	May 12 2023, 9:04 PM

	F36994618: Screenshot 2023-05-12 at 1.58.04 PM.png
	May 12 2023, 9:04 PM

	F36976259: Screen Recording 2023-05-04 at 8.16.40 PM.mov.gif
	May 5 2023, 3:27 AM

	F36971810: Screen Recording 2023-05-01 at 2.00.34 PM.mov.gif
	May 1 2023, 9:02 PM

	F36971808: Screen Recording 2023-05-01 at 1.55.53 PM.mov.gif
	May 1 2023, 9:02 PM

	F36971806: Screenshot 2023-05-01 at 1.55.15 PM.png
	May 1 2023, 9:02 PM

popupNotification Fix memory leaksClosed, ResolvedPublic2 Estimated Story PointsBUG REPORTActions

Description

QA

QA Results - Beta

QA Results - Prod

Details

Related Objects

Event Timeline

Test Result - Beta

QA Steps

Test Result - Beta

QA Steps

Test Result - Prod

QA Steps

popupNotification Fix memory leaks
Closed, ResolvedPublic2 Estimated Story PointsBUG REPORT
Actions