From c7d98ebe5a32c88d6b46aaf2646db86aa74a0baa Mon Sep 17 00:00:00 2001 From: Dana Estra Date: Thu, 11 Jul 2024 13:45:46 -0700 Subject: [PATCH 1/2] YouTube: Unable to move the volume slider by dragging after disabling Video Viewer https://bugs.webkit.org/show_bug.cgi?id=276496 rdar://128587639 Reviewed by Eric Carlson. When entering fullscreen, we add a dragstart event listener to the window that we never remove. This makes it so that after we have exited fullscreen, the dragstart event listener is still active, and it has a side effect of making the elements on the page to be draggable. This is noticeable when the user is moving youtube's volume slider. Instead of this moving the knob on the slider, this drags an image of the slider. To fix this, we should remove the dragstart event listener on the window when we exit fullscreen. This will remove the event listener's side effect that is causing this bug. * Source/WebCore/Modules/modern-media-controls/controls/media-controls.js: (MediaControls.prototype.disable): (MediaControls): * Source/WebCore/Modules/modern-media-controls/media/media-controller.js: (MediaController.prototype._updateControlsIfNeeded): Canonical link: https://commits.webkit.org/280873@main --- .../modern-media-controls/controls/media-controls.js | 6 ++++++ .../Modules/modern-media-controls/media/media-controller.js | 3 +++ 2 files changed, 9 insertions(+) diff --git a/Source/WebCore/Modules/modern-media-controls/controls/media-controls.js b/Source/WebCore/Modules/modern-media-controls/controls/media-controls.js index da549e80c6a34..e07713447ce74 100644 --- a/Source/WebCore/Modules/modern-media-controls/controls/media-controls.js +++ b/Source/WebCore/Modules/modern-media-controls/controls/media-controls.js @@ -248,4 +248,10 @@ class MediaControls extends LayoutNode else super.commitProperty(propertyName); } + + disable() + { + this.element.removeEventListener("focusin", this); + window.removeEventListener("dragstart", this, true); + } } diff --git a/Source/WebCore/Modules/modern-media-controls/media/media-controller.js b/Source/WebCore/Modules/modern-media-controls/media/media-controller.js index 8ed8708690836..af91135135b0b 100644 --- a/Source/WebCore/Modules/modern-media-controls/media/media-controller.js +++ b/Source/WebCore/Modules/modern-media-controls/media/media-controller.js @@ -269,6 +269,9 @@ class MediaController supportingObject.disable(); } + if (previousControls) + previousControls.disable(); + this.controls = new ControlsClass; this.controls.delegate = this; From 73f49fdda1f6eeb39c516f5e5fb3e58f79769f9b Mon Sep 17 00:00:00 2001 From: Xabier Rodriguez-Calvar Date: Sun, 29 Sep 2024 22:29:28 -0700 Subject: [PATCH 2/2] [Modern Media Controls] remove window event listeners at deinitialization https://bugs.webkit.org/show_bug.cgi?id=280503 Reviewed by Ryan Reno. DOMWindow keeps media controls object alive because of installed event listeners (see JSDOMWindow::visitAdditionalChildren). This results in extra memory usage undil DOMWindow cleared (on top level navigation). Original patch by Eugene Mutavchi . * Source/WebCore/Modules/modern-media-controls/controls/media-controls.js: (MediaControls.prototype.reenable): * Source/WebCore/Modules/modern-media-controls/media/media-controller.js: (MediaController.prototype.deinitialize): (MediaController.prototype.reinitialize): Canonical link: https://commits.webkit.org/284440@main --- .../modern-media-controls/controls/media-controls.js | 6 ++++++ .../Modules/modern-media-controls/media/media-controller.js | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/Source/WebCore/Modules/modern-media-controls/controls/media-controls.js b/Source/WebCore/Modules/modern-media-controls/controls/media-controls.js index e07713447ce74..679191d4e46f3 100644 --- a/Source/WebCore/Modules/modern-media-controls/controls/media-controls.js +++ b/Source/WebCore/Modules/modern-media-controls/controls/media-controls.js @@ -254,4 +254,10 @@ class MediaControls extends LayoutNode this.element.removeEventListener("focusin", this); window.removeEventListener("dragstart", this, true); } + + reenable() + { + this.element.addEventListener("focusin", this); + window.addEventListener("dragstart", this, true); + } } diff --git a/Source/WebCore/Modules/modern-media-controls/media/media-controller.js b/Source/WebCore/Modules/modern-media-controls/media/media-controller.js index af91135135b0b..13f0450736f1a 100644 --- a/Source/WebCore/Modules/modern-media-controls/media/media-controller.js +++ b/Source/WebCore/Modules/modern-media-controls/media/media-controller.js @@ -222,6 +222,9 @@ class MediaController deinitialize() { this.shadowRoot.removeChild(this.container); + window.removeEventListener("keydown", this); + if (this.controls) + this.controls.disable(); return true; } @@ -232,6 +235,9 @@ class MediaController this.mediaWeakRef = new WeakRef(media); this.host = host; shadowRoot.appendChild(this.container); + window.addEventListener("keydown", this); + if (this.controls) + this.controls.reenable(); return true; }