Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove media container observers on disconnect #1061

Merged
merged 1 commit into from
Jan 28, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
202 changes: 105 additions & 97 deletions src/js/media-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import { globalThis, document } from './utils/server-safe-globals.js';
import { MediaUIAttributes, MediaStateChangeEvents } from './constants.js';
import { nouns } from './labels/labels.js';
import { observeResize } from './utils/resize-observer.js';
import { observeResize, unobserveResize } from './utils/resize-observer.js';
// Guarantee that `<media-gesture-receiver/>` is available for use in the template
import './media-gesture-receiver.js';

Expand Down Expand Up @@ -89,11 +89,11 @@
*/ ''
}
:host(:not([${Attributes.AUDIO}])[${
Attributes.GESTURES_DISABLED
}]) ::slotted([slot=gestures-chrome]),
Attributes.GESTURES_DISABLED
}]) ::slotted([slot=gestures-chrome]),
:host(:not([${Attributes.AUDIO}])[${
Attributes.GESTURES_DISABLED
}]) media-gesture-receiver[slot=gestures-chrome] {
Attributes.GESTURES_DISABLED
}]) media-gesture-receiver[slot=gestures-chrome] {
display: none;
}

Expand Down Expand Up @@ -160,23 +160,23 @@
/* Hide controls when inactive, not paused, not audio and auto hide not disabled */ ''
}
:host([${Attributes.USER_INACTIVE}]:not([${
MediaUIAttributes.MEDIA_PAUSED
}]):not([${MediaUIAttributes.MEDIA_IS_AIRPLAYING}]):not([${
MediaUIAttributes.MEDIA_IS_CASTING
}]):not([${
Attributes.AUDIO
}])) ::slotted(:not([slot=media]):not([slot=poster]):not([${
Attributes.NO_AUTOHIDE
}]):not([role=dialog])) {
MediaUIAttributes.MEDIA_PAUSED
}]):not([${MediaUIAttributes.MEDIA_IS_AIRPLAYING}]):not([${
MediaUIAttributes.MEDIA_IS_CASTING
}]):not([${
Attributes.AUDIO
}])) ::slotted(:not([slot=media]):not([slot=poster]):not([${
Attributes.NO_AUTOHIDE
}]):not([role=dialog])) {
opacity: 0;
transition: opacity 1s;
}

:host([${Attributes.USER_INACTIVE}]:not([${
MediaUIAttributes.MEDIA_PAUSED
}]):not([${MediaUIAttributes.MEDIA_IS_CASTING}]):not([${
Attributes.AUDIO
}])) ::slotted([slot=media]) {
MediaUIAttributes.MEDIA_PAUSED
}]):not([${MediaUIAttributes.MEDIA_IS_CASTING}]):not([${
Attributes.AUDIO
}])) ::slotted([slot=media]) {
cursor: none;
}

Expand All @@ -188,8 +188,8 @@
/* ::slotted([slot=poster]) doesn't work for slot fallback content so hide parent slot instead */ ''
}
:host(:not([${Attributes.AUDIO}])[${
MediaUIAttributes.MEDIA_HAS_PLAYED
}]) slot[name=poster] {
MediaUIAttributes.MEDIA_HAS_PLAYED
}]) slot[name=poster] {
display: none;
}

Expand Down Expand Up @@ -323,84 +323,6 @@
this.shadowRoot.appendChild(template.content.cloneNode(true));
}

// Watch for child adds/removes and update the media element if necessary
const mutationCallback = (mutationsList: MutationRecord[]) => {
const media = this.media;

for (const mutation of mutationsList) {
if (mutation.type === 'childList') {
// Media element being removed
mutation.removedNodes.forEach((node: Element) => {
// Is this a direct child media element of media-controller?
// TODO: This accuracy doesn't matter after moving away from media attrs.
// Could refactor so we can always just call 'dispose' on any removed media el.
if (node.slot == 'media' && mutation.target == this) {
// Check if this was the current media by if it was the first
// el with slot=media in the child list. There could be multiple.
let previousSibling =
mutation.previousSibling &&
(mutation.previousSibling as Element).previousElementSibling;

// Must have been first if no prev sibling or new media
if (!previousSibling || !media) {
this.mediaUnsetCallback(node as HTMLMediaElement);
} else {
// Check if any prev siblings had a slot=media
// Should remain true otherwise
let wasFirst = previousSibling.slot !== 'media';
while (
(previousSibling =
previousSibling.previousSibling as Element) !== null
) {
if (previousSibling.slot == 'media') wasFirst = false;
}
if (wasFirst) this.mediaUnsetCallback(node as HTMLMediaElement);
}
}
});

// Controls or media element being added
// No need to inject anything if media=null
if (media) {
mutation.addedNodes.forEach((node) => {
if (node === media) {
// Update all controls with new media if this is the new media
this.handleMediaUpdated(media);
}
});
}
}
}
};

const mutationObserver = new MutationObserver(mutationCallback);
mutationObserver.observe(this, { childList: true, subtree: true });

let pendingResizeCb = false;
const deferResizeCallback = (entry: ResizeObserverEntry) => {
// Already have a pending async breakpoint computation, so go ahead and bail
if (pendingResizeCb) return;
// Just in case it takes too long (which will cause an error to throw),
// do the breakpoint computation asynchronously
setTimeout(() => {
resizeCallback(entry);
// Once we've completed, reset the pending cb flag to false
pendingResizeCb = false;

if (!this.breakpointsComputed) {
this.breakpointsComputed = true;
this.dispatchEvent(
new CustomEvent(MediaStateChangeEvents.BREAKPOINTS_COMPUTED, {
bubbles: true,
composed: true,
})
);
}
}, 0);
pendingResizeCb = true;
};
observeResize(this, deferResizeCallback);

// Handles the case when the slotted media element is a slot element itself.
// e.g. chaining media slots for media themes.
const chainedSlot = this.querySelector(
Expand Down Expand Up @@ -462,6 +384,10 @@
}

connectedCallback(): void {
// Watch for child adds/removes and update the media element if necessary
this.#mutationObserver.observe(this, { childList: true, subtree: true });
observeResize(this, this.#handleResize);

const isAudioChrome = this.getAttribute(Attributes.AUDIO) != null;
const label = isAudioChrome ? nouns.AUDIO_PLAYER() : nouns.VIDEO_PLAYER();
this.setAttribute('role', 'region');
Expand All @@ -483,6 +409,9 @@
}

disconnectedCallback(): void {
this.#mutationObserver.disconnect();
unobserveResize(this, this.#handleResize);

// When disconnected from the DOM, remove root node and media event listeners
// to prevent memory leaks and unneeded invisble UI updates.
if (this.media) {
Expand Down Expand Up @@ -528,6 +457,85 @@
}
}

#mutationObserver = new MutationObserver(this.#handleMutation.bind(this));
#handleMutation(mutationsList: MutationRecord[]) {
const media = this.media;

for (const mutation of mutationsList) {
if (mutation.type !== 'childList') continue;

const removedNodes = mutation.removedNodes as NodeListOf<Element>;

// Media element being removed
for (const node of removedNodes) {
// Is this a direct child media element of media-controller?
// TODO: This accuracy doesn't matter after moving away from media attrs.
// Could refactor so we can always just call 'dispose' on any removed media el.
if (node.slot != 'media' || mutation.target != this) continue;

// Check if this was the current media by if it was the first
// el with slot=media in the child list. There could be multiple.
let previousSibling =
mutation.previousSibling &&
(mutation.previousSibling as Element).previousElementSibling;

Check warning on line 480 in src/js/media-container.ts

View check run for this annotation

Codecov / codecov/patch

src/js/media-container.ts#L480

Added line #L480 was not covered by tests

// Must have been first if no prev sibling or new media
if (!previousSibling || !media) {
this.mediaUnsetCallback(node as HTMLMediaElement);
} else {
// Check if any prev siblings had a slot=media
// Should remain true otherwise
let wasFirst = previousSibling.slot !== 'media';

while (
(previousSibling = previousSibling.previousSibling as Element) !==
null
) {
if (previousSibling.slot == 'media') wasFirst = false;
}

if (wasFirst) this.mediaUnsetCallback(node as HTMLMediaElement);
}

Check warning on line 498 in src/js/media-container.ts

View check run for this annotation

Codecov / codecov/patch

src/js/media-container.ts#L486-L498

Added lines #L486 - L498 were not covered by tests
}

// Controls or media element being added
// No need to inject anything if media=null
if (media) {
for (const node of mutation.addedNodes) {
// Update all controls with new media if this is the new media
if (node === media) this.handleMediaUpdated(media);
}
}
}
}

#isResizePending = false;
#handleResize = (entry: ResizeObserverEntry) => {
// Already have a pending async breakpoint computation, so go ahead and bail
if (this.#isResizePending) return;

// Just in case it takes too long (which will cause an error to throw),
// do the breakpoint computation asynchronously
setTimeout(() => {
resizeCallback(entry);

// Once we've completed, reset the pending cb flag to false
this.#isResizePending = false;

if (!this.breakpointsComputed) {
this.breakpointsComputed = true;
this.dispatchEvent(
new CustomEvent(MediaStateChangeEvents.BREAKPOINTS_COMPUTED, {
bubbles: true,
composed: true,
})
);
}
}, 0);

this.#isResizePending = true;
};

#handlePointerMove(event: PointerEvent) {
if (event.pointerType !== 'mouse') {
// On mobile we toggle the controls on a tap which is handled in pointerup,
Expand Down
Loading