From 06b5422abf70a69d6e3eed4fc9e414ed4ec3696e Mon Sep 17 00:00:00 2001 From: Andrei Volykhin Date: Thu, 19 Jun 2025 12:55:10 +0300 Subject: [PATCH] htmlvideoelement: Fix poster frame processing algorithm (#37533) According to HTML specification the poster attribute determines the element's poster frame (regardless of the value of the element's show poster flag). https://html.spec.whatwg.org/multipage/#poster-frame So the poster frame and the show poster flag is orthogonal to each other, the latest one only controls when browser should display the poster frame and should do not block accepting video frames from media pipeline (e.g. on new_preroll callback from video sink). During layout the video element should select the current frame which will be presented based on first matching condition from the list: https://html.spec.whatwg.org/multipage/#the-video-element:the-video-element-7 Testing: Improvements in the following WPT tests - html/canvas/element/manual/imagebitmap/createImageBitmap* - html/semantics/embedded-content/the-canvas-element/* Fixes: #37165 --------- Signed-off-by: Andrei Volykhin Signed-off-by: Martin Robinson Co-authored-by: Martin Robinson --- components/script/dom/htmlmediaelement.rs | 77 +++++++++---------- components/script/dom/htmlvideoelement.rs | 48 ++++++++---- ...eImageBitmap-colorSpaceConversion.html.ini | 3 - .../createImageBitmap-origin.sub.html.ini | 18 ----- ...ateImageBitmap-resolves-in-task.any.js.ini | 6 -- .../createImageBitmap-serializable.html.ini | 6 -- .../createImageBitmap-transfer.html.ini | 6 -- .../security.pattern.fillStyle.sub.html.ini | 18 ----- 8 files changed, 70 insertions(+), 112 deletions(-) diff --git a/components/script/dom/htmlmediaelement.rs b/components/script/dom/htmlmediaelement.rs index 3d6d8f2938d..060b5358a8c 100644 --- a/components/script/dom/htmlmediaelement.rs +++ b/components/script/dom/htmlmediaelement.rs @@ -170,7 +170,8 @@ pub(crate) struct MediaFrameRenderer { old_frame: Option, very_old_frame: Option, current_frame_holder: Option, - show_poster: bool, + /// + poster_frame: Option, } impl MediaFrameRenderer { @@ -182,29 +183,23 @@ impl MediaFrameRenderer { old_frame: None, very_old_frame: None, current_frame_holder: None, - show_poster: false, + poster_frame: None, } } - fn render_poster_frame(&mut self, image: Arc) { - if let Some(image_key) = image.id { - self.current_frame = Some(MediaFrame { + fn set_poster_frame(&mut self, image: Option>) { + self.poster_frame = image.and_then(|image| { + image.id.map(|image_key| MediaFrame { image_key, width: image.metadata.width as i32, height: image.metadata.height as i32, - }); - self.show_poster = true; - } + }) + }); } } impl VideoFrameRenderer for MediaFrameRenderer { fn render(&mut self, frame: VideoFrame) { - // Don't render new frames if the poster should be shown - if self.show_poster { - return; - } - let mut updates = vec![]; if let Some(old_image_key) = mem::replace(&mut self.very_old_frame, self.old_frame.take()) { @@ -730,7 +725,7 @@ impl HTMLMediaElement { self.paused.set(false); // Step 2 if self.show_poster.get() { - self.set_show_poster(false); + self.show_poster.set(false); self.time_marches_on(); } // Step 3 @@ -753,7 +748,7 @@ impl HTMLMediaElement { self.network_state.set(NetworkState::NoSource); // Step 2. - self.set_show_poster(true); + self.show_poster.set(true); // Step 3. self.delay_load_event(true, can_gc); @@ -1081,7 +1076,7 @@ impl HTMLMediaElement { this.network_state.set(NetworkState::NoSource); // Step 4. - this.set_show_poster(true); + this.show_poster.set(true); // Step 5. this.upcast::().fire_event(atom!("error"), CanGc::note()); @@ -1198,8 +1193,8 @@ impl HTMLMediaElement { self.fulfill_in_flight_play_promises(|| ()); } - // Step 6.7. - if !self.seeking.get() { + // Step 6.7. If seeking is true, set it to false. + if self.seeking.get() { self.seeking.set(false); } @@ -1298,7 +1293,7 @@ impl HTMLMediaElement { // https://html.spec.whatwg.org/multipage/#dom-media-seek fn seek(&self, time: f64, _approximate_for_speed: bool) { // Step 1. - self.set_show_poster(false); + self.show_poster.set(false); // Step 2. if self.ready_state.get() == ReadyState::HaveNothing { @@ -1403,19 +1398,14 @@ impl HTMLMediaElement { } /// - pub(crate) fn process_poster_image_loaded(&self, image: Arc) { - if !self.show_poster.get() { - return; - } + pub(crate) fn set_poster_frame(&self, image: Option>) { + let queue_postershown_event = pref!(media_testing_enabled) && image.is_some(); - // Step 6. - self.handle_resize(Some(image.metadata.width), Some(image.metadata.height)); - self.video_renderer - .lock() - .unwrap() - .render_poster_frame(image); + self.video_renderer.lock().unwrap().set_poster_frame(image); - if pref!(media_testing_enabled) { + self.upcast::().dirty(NodeDamage::Other); + + if queue_postershown_event { self.owner_global() .task_manager() .media_element_task_source() @@ -2097,6 +2087,7 @@ impl HTMLMediaElement { } } + /// Gets the video frame at the current playback position. pub(crate) fn get_current_frame(&self) -> Option { self.video_renderer .lock() @@ -2106,8 +2097,21 @@ impl HTMLMediaElement { .map(|holder| holder.get_frame()) } - pub(crate) fn get_current_frame_data(&self) -> Option { - self.video_renderer.lock().unwrap().current_frame + /// Gets the current frame of the video element to present, if any. + /// + pub(crate) fn get_current_frame_to_present(&self) -> Option { + let (current_frame, poster_frame) = { + let renderer = self.video_renderer.lock().unwrap(); + (renderer.current_frame, renderer.poster_frame) + }; + + // If the show poster flag is set (or there is no current video frame to + // present) AND there is a poster frame, present that. + if (self.show_poster.get() || current_frame.is_none()) && poster_frame.is_some() { + return poster_frame; + } + + current_frame } pub(crate) fn clear_current_frame_data(&self) { @@ -2153,13 +2157,6 @@ impl HTMLMediaElement { self.duration.set(duration); } - /// Sets a new value for the show_poster propperty. Updates video_rederer - /// with the new value. - pub(crate) fn set_show_poster(&self, show_poster: bool) { - self.show_poster.set(show_poster); - self.video_renderer.lock().unwrap().show_poster = show_poster; - } - pub(crate) fn reset(&self) { if let Some(ref player) = *self.player.borrow() { if let Err(e) = player.lock().unwrap().stop() { @@ -2378,7 +2375,7 @@ impl HTMLMediaElementMethods for HTMLMediaElement { // Step 6.2. if self.show_poster.get() { - self.set_show_poster(false); + self.show_poster.set(false); self.time_marches_on(); } diff --git a/components/script/dom/htmlvideoelement.rs b/components/script/dom/htmlvideoelement.rs index 622e5079ee9..8c03b5f8c91 100644 --- a/components/script/dom/htmlvideoelement.rs +++ b/components/script/dom/htmlvideoelement.rs @@ -133,6 +133,9 @@ impl HTMLVideoElement { sent_resize } + /// Gets the copy of the video frame at the current playback position, + /// if that is available, or else (e.g. when the video is seeking or buffering) + /// its previous appearance, if any. pub(crate) fn get_current_frame_data(&self) -> Option { let frame = self.htmlmediaelement.get_current_frame(); if frame.is_some() { @@ -163,22 +166,31 @@ impl HTMLVideoElement { } /// - fn fetch_poster_frame(&self, poster_url: &str, can_gc: CanGc) { - // Step 1. + fn update_poster_frame(&self, poster_url: Option<&str>, can_gc: CanGc) { + // Step 1. If there is an existing instance of this algorithm running + // for this video element, abort that instance of this algorithm without + // changing the poster frame. self.generation_id.set(self.generation_id.get() + 1); - // Step 2. - if poster_url.is_empty() { + // Step 2. If the poster attribute's value is the empty string or + // if the attribute is absent, then there is no poster frame; return. + if poster_url.is_none_or(|url| url.is_empty()) { + self.htmlmediaelement.set_poster_frame(None); return; } - // Step 3. - let poster_url = match self.owner_document().url().join(poster_url) { + // Step 3. Let url be the result of encoding-parsing a URL given + // the poster attribute's value, relative to the element's node + // document. + // Step 4. If url is failure, then return. There is no poster frame. + let poster_url = match self.owner_document().url().join(poster_url.unwrap()) { Ok(url) => url, - Err(_) => return, + Err(_) => { + self.htmlmediaelement.set_poster_frame(None); + return; + }, }; - // Step 4. // We use the image cache for poster frames so we save as much // network activity as possible. let window = self.owner_window(); @@ -228,7 +240,9 @@ impl HTMLVideoElement { /// fn do_fetch_poster_frame(&self, poster_url: ServoUrl, id: PendingImageId, can_gc: CanGc) { - // Continuation of step 4. + // Step 5. Let request be a new request whose URL is url, client is the element's node + // document's relevant settings object, destination is "image", initiator type is "video", + // credentials mode is "include", and whose use-URL-credentials flag is set. let document = self.owner_document(); let request = RequestBuilder::new( Some(document.webview_id()), @@ -243,7 +257,8 @@ impl HTMLVideoElement { .insecure_requests_policy(document.insecure_requests_policy()) .has_trustworthy_ancestor_origin(document.has_trustworthy_ancestor_origin()) .policy_container(document.policy_container().to_owned()); - // Step 5. + + // Step 6. Fetch request. This must delay the load event of the element's node document. // This delay must be independent from the ones created by HTMLMediaElement during // its media load algorithm, otherwise a code like // @@ -264,12 +279,15 @@ impl HTMLVideoElement { self.generation_id.get() } + /// fn process_image_response(&self, response: ImageResponse, can_gc: CanGc) { + // Step 7. If an image is thus obtained, the poster frame is that image. + // Otherwise, there is no poster frame. match response { ImageResponse::Loaded(image, url) => { debug!("Loaded poster image for video element: {:?}", url); match image.as_raster_image() { - Some(image) => self.htmlmediaelement.process_poster_image_loaded(image), + Some(image) => self.htmlmediaelement.set_poster_frame(Some(image)), None => warn!("Vector images are not yet supported in video poster"), } LoadBlocker::terminate(&self.load_blocker, can_gc); @@ -277,6 +295,7 @@ impl HTMLVideoElement { ImageResponse::MetadataLoaded(..) => {}, // The image cache may have loaded a placeholder for an invalid poster url ImageResponse::PlaceholderLoaded(..) | ImageResponse::None => { + self.htmlmediaelement.set_poster_frame(None); // A failed load should unblock the document load. LoadBlocker::terminate(&self.load_blocker, can_gc); }, @@ -340,10 +359,9 @@ impl VirtualMethods for HTMLVideoElement { if attr.local_name() == &local_name!("poster") { if let Some(new_value) = mutation.new_value(attr) { - self.fetch_poster_frame(&new_value, CanGc::note()) + self.update_poster_frame(Some(&new_value), CanGc::note()) } else { - self.htmlmediaelement.clear_current_frame_data(); - self.htmlmediaelement.set_show_poster(false); + self.update_poster_frame(None, CanGc::note()) } }; } @@ -521,7 +539,7 @@ impl LayoutHTMLVideoElementHelpers for LayoutDom<'_, HTMLVideoElement> { let video = self.unsafe_get(); // Get the current frame being rendered. - let current_frame = video.htmlmediaelement.get_current_frame_data(); + let current_frame = video.htmlmediaelement.get_current_frame_to_present(); // This value represents the natural width and height of the video. // It may exist even if there is no current frame (for example, after the diff --git a/tests/wpt/meta/html/canvas/element/manual/imagebitmap/createImageBitmap-colorSpaceConversion.html.ini b/tests/wpt/meta/html/canvas/element/manual/imagebitmap/createImageBitmap-colorSpaceConversion.html.ini index 3456a9eac35..ab8397b7990 100644 --- a/tests/wpt/meta/html/canvas/element/manual/imagebitmap/createImageBitmap-colorSpaceConversion.html.ini +++ b/tests/wpt/meta/html/canvas/element/manual/imagebitmap/createImageBitmap-colorSpaceConversion.html.ini @@ -1,6 +1,3 @@ [createImageBitmap-colorSpaceConversion.html] [createImageBitmap from a Blob, and drawImage on the created ImageBitmap with colorSpaceConversion: none] expected: FAIL - - [createImageBitmap from a Video, and drawImage on the created ImageBitmap with colorSpaceConversion: none] - expected: FAIL diff --git a/tests/wpt/meta/html/canvas/element/manual/imagebitmap/createImageBitmap-origin.sub.html.ini b/tests/wpt/meta/html/canvas/element/manual/imagebitmap/createImageBitmap-origin.sub.html.ini index 0b2c6bcd0ff..22ec677f4ea 100644 --- a/tests/wpt/meta/html/canvas/element/manual/imagebitmap/createImageBitmap-origin.sub.html.ini +++ b/tests/wpt/meta/html/canvas/element/manual/imagebitmap/createImageBitmap-origin.sub.html.ini @@ -1,45 +1,27 @@ [createImageBitmap-origin.sub.html] - [redirected to cross-origin HTMLVideoElement: origin unclear 2dContext.drawImage] - expected: FAIL - [redirected to cross-origin HTMLVideoElement: origin unclear bitmaprenderer.transferFromImageBitmap] expected: FAIL [unclean HTMLCanvasElement: origin unclear bitmaprenderer.transferFromImageBitmap] expected: FAIL - [cross-origin HTMLVideoElement: origin unclear getImageData] - expected: FAIL - [cross-origin SVGImageElement: origin unclear bitmaprenderer.transferFromImageBitmap] expected: FAIL [cross-origin HTMLVideoElement: origin unclear bitmaprenderer.transferFromImageBitmap] expected: FAIL - [redirected to same-origin HTMLVideoElement: origin unclear getImageData] - expected: FAIL - [cross-origin SVGImageElement: origin unclear 2dContext.drawImage] expected: FAIL [cross-origin HTMLImageElement: origin unclear bitmaprenderer.transferFromImageBitmap] expected: FAIL - [redirected to same-origin HTMLVideoElement: origin unclear 2dContext.drawImage] - expected: FAIL - [unclean ImageBitmap: origin unclear bitmaprenderer.transferFromImageBitmap] expected: FAIL [redirected to same-origin HTMLVideoElement: origin unclear bitmaprenderer.transferFromImageBitmap] expected: FAIL - [redirected to cross-origin HTMLVideoElement: origin unclear getImageData] - expected: FAIL - - [cross-origin HTMLVideoElement: origin unclear 2dContext.drawImage] - expected: FAIL - [cross-origin SVGImageElement: origin unclear getImageData] expected: FAIL diff --git a/tests/wpt/meta/html/canvas/element/manual/imagebitmap/createImageBitmap-resolves-in-task.any.js.ini b/tests/wpt/meta/html/canvas/element/manual/imagebitmap/createImageBitmap-resolves-in-task.any.js.ini index d414b0aa93f..f94f75164bf 100644 --- a/tests/wpt/meta/html/canvas/element/manual/imagebitmap/createImageBitmap-resolves-in-task.any.js.ini +++ b/tests/wpt/meta/html/canvas/element/manual/imagebitmap/createImageBitmap-resolves-in-task.any.js.ini @@ -1,10 +1,4 @@ [createImageBitmap-resolves-in-task.any.html] - [createImageBitmap with an HTMLVideoElement source should resolve async] - expected: FAIL - - [createImageBitmap with an HTMLVideoElement from a data URL source should resolve async] - expected: FAIL - [createImageBitmap with a vector HTMLImageElement source should resolve async] expected: FAIL diff --git a/tests/wpt/meta/html/canvas/element/manual/imagebitmap/createImageBitmap-serializable.html.ini b/tests/wpt/meta/html/canvas/element/manual/imagebitmap/createImageBitmap-serializable.html.ini index fd32af77cd5..e710fa8be3e 100644 --- a/tests/wpt/meta/html/canvas/element/manual/imagebitmap/createImageBitmap-serializable.html.ini +++ b/tests/wpt/meta/html/canvas/element/manual/imagebitmap/createImageBitmap-serializable.html.ini @@ -2,12 +2,6 @@ [Serialize ImageBitmap created from a vector SVGImageElement] expected: FAIL - [Serialize ImageBitmap created from an HTMLVideoElement] - expected: FAIL - - [Serialize ImageBitmap created from an HTMLVideoElement from a data URL] - expected: FAIL - [Serialize ImageBitmap created from a vector HTMLImageElement] expected: FAIL diff --git a/tests/wpt/meta/html/canvas/element/manual/imagebitmap/createImageBitmap-transfer.html.ini b/tests/wpt/meta/html/canvas/element/manual/imagebitmap/createImageBitmap-transfer.html.ini index 1c8de6ec5a9..a0c2c0b3a83 100644 --- a/tests/wpt/meta/html/canvas/element/manual/imagebitmap/createImageBitmap-transfer.html.ini +++ b/tests/wpt/meta/html/canvas/element/manual/imagebitmap/createImageBitmap-transfer.html.ini @@ -8,11 +8,5 @@ [Transfer ImageBitmap created from a Blob] expected: FAIL - [Transfer ImageBitmap created from an HTMLVideoElement from a data URL] - expected: FAIL - [Transfer ImageBitmap created from a bitmap SVGImageElement] expected: FAIL - - [Transfer ImageBitmap created from an HTMLVideoElement] - expected: FAIL diff --git a/tests/wpt/meta/html/semantics/embedded-content/the-canvas-element/security.pattern.fillStyle.sub.html.ini b/tests/wpt/meta/html/semantics/embedded-content/the-canvas-element/security.pattern.fillStyle.sub.html.ini index ac0696f9b90..3a3b73a56cb 100644 --- a/tests/wpt/meta/html/semantics/embedded-content/the-canvas-element/security.pattern.fillStyle.sub.html.ini +++ b/tests/wpt/meta/html/semantics/embedded-content/the-canvas-element/security.pattern.fillStyle.sub.html.ini @@ -2,23 +2,5 @@ [cross-origin SVGImageElement: Setting fillStyle to an origin-unclean pattern makes the canvas origin-unclean] expected: FAIL - [cross-origin HTMLVideoElement: Setting fillStyle to an origin-unclean pattern makes the canvas origin-unclean] - expected: FAIL - - [redirected to cross-origin HTMLVideoElement: Setting fillStyle to an origin-unclean pattern makes the canvas origin-unclean] - expected: FAIL - - [redirected to same-origin HTMLVideoElement: Setting fillStyle to an origin-unclean pattern makes the canvas origin-unclean] - expected: FAIL - [cross-origin SVGImageElement: Setting fillStyle to an origin-unclean offscreen canvas pattern makes the canvas origin-unclean] expected: FAIL - - [cross-origin HTMLVideoElement: Setting fillStyle to an origin-unclean offscreen canvas pattern makes the canvas origin-unclean] - expected: FAIL - - [redirected to cross-origin HTMLVideoElement: Setting fillStyle to an origin-unclean offscreen canvas pattern makes the canvas origin-unclean] - expected: FAIL - - [redirected to same-origin HTMLVideoElement: Setting fillStyle to an origin-unclean offscreen canvas pattern makes the canvas origin-unclean] - expected: FAIL