From 972ca77ce1bd2c08b809b7d54ec716213519fe1e Mon Sep 17 00:00:00 2001 From: TIN TUN AUNG <62133983+rayguo17@users.noreply.github.com> Date: Fri, 11 Apr 2025 14:32:34 +0800 Subject: [PATCH] dom: should change media element's currentSrc to children source element's src in resource selection algorithm. (#36408) Set the `htmlmediaelement`'s `currenSrc` in resource-selection-algorithm. Change the `htmlsourceelement`'s src and srcset to USVString type. According to [Spec](https://html.spec.whatwg.org/multipage/media.html#concept-media-load-algorithm), Step 9.3 for mode is children, should set the `currentSrc` to `src` of children `htmlsourceelement`. Also, In the `htmlsourceelement` [interface definition](https://html.spec.whatwg.org/multipage/embedded-content.html#the-source-element), the `src` and `srcset` attribute should be type `USVString`. Testing: More WPT tests related to resource selection algorithm are passing. Fix: Some spec fix [Try](https://github.com/rayguo17/servo/actions/runs/14347535616) cc @xiaochengh Signed-off-by: rayguo17 --- components/script/dom/htmlmediaelement.rs | 24 ++++++++++++------- components/script/dom/htmlsourceelement.rs | 10 ++++---- .../webidls/HTMLSourceElement.webidl | 4 ++-- .../dynamic-urls.sub.html.ini | 3 --- .../historical.sub.xhtml.ini | 3 --- ...selection-candidate-insert-before.html.ini | 4 ---- .../currentSrc.html.ini | 19 --------------- 7 files changed, 22 insertions(+), 45 deletions(-) delete mode 100644 tests/wpt/meta/html/semantics/embedded-content/media-elements/loading-the-media-resource/resource-selection-candidate-insert-before.html.ini delete mode 100644 tests/wpt/meta/html/semantics/embedded-content/media-elements/location-of-the-media-resource/currentSrc.html.ini diff --git a/components/script/dom/htmlmediaelement.rs b/components/script/dom/htmlmediaelement.rs index b16c17f1d79..1c802b38f72 100644 --- a/components/script/dom/htmlmediaelement.rs +++ b/components/script/dom/htmlmediaelement.rs @@ -52,7 +52,6 @@ use crate::dom::bindings::codegen::Bindings::AttrBinding::AttrMethods; use crate::dom::bindings::codegen::Bindings::HTMLMediaElementBinding::{ CanPlayTypeResult, HTMLMediaElementConstants, HTMLMediaElementMethods, }; -use crate::dom::bindings::codegen::Bindings::HTMLSourceElementBinding::HTMLSourceElementMethods; use crate::dom::bindings::codegen::Bindings::MediaErrorBinding::MediaErrorConstants::*; use crate::dom::bindings::codegen::Bindings::MediaErrorBinding::MediaErrorMethods; use crate::dom::bindings::codegen::Bindings::NavigatorBinding::Navigator_Binding::NavigatorMethods; @@ -836,15 +835,20 @@ impl HTMLMediaElement { Mode::Children(source) => { // This is only a partial implementation // FIXME: https://github.com/servo/servo/issues/21481 - let src = source.Src(); + let src = source + .upcast::() + .get_attribute(&ns!(), &local_name!("src")); // Step 9.attr.2. - if src.is_empty() { - source - .upcast::() - .fire_event(atom!("error"), can_gc); - self.queue_dedicated_media_source_failure_steps(); - return; - } + let src: String = match src { + Some(src) if !src.Value().is_empty() => src.Value().into(), + _ => { + source + .upcast::() + .fire_event(atom!("error"), can_gc); + self.queue_dedicated_media_source_failure_steps(); + return; + }, + }; // Step 9.attr.3. let url_record = match base_url.join(&src) { Ok(url) => url, @@ -856,6 +860,8 @@ impl HTMLMediaElement { return; }, }; + // Step 9.attr.7 + *self.current_src.borrow_mut() = url_record.as_str().into(); // Step 9.attr.8. self.resource_fetch_algorithm(Resource::Url(url_record)); }, diff --git a/components/script/dom/htmlsourceelement.rs b/components/script/dom/htmlsourceelement.rs index 45772e4bac5..e4b115e78a2 100644 --- a/components/script/dom/htmlsourceelement.rs +++ b/components/script/dom/htmlsourceelement.rs @@ -11,7 +11,7 @@ use crate::dom::bindings::codegen::Bindings::HTMLSourceElementBinding::HTMLSourc use crate::dom::bindings::codegen::Bindings::NodeBinding::Node_Binding::NodeMethods; use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::root::{Dom, DomRoot, Root}; -use crate::dom::bindings::str::DOMString; +use crate::dom::bindings::str::{DOMString, USVString}; use crate::dom::document::Document; use crate::dom::element::AttributeMutation; use crate::dom::htmlelement::HTMLElement; @@ -119,10 +119,10 @@ impl VirtualMethods for HTMLSourceElement { impl HTMLSourceElementMethods for HTMLSourceElement { // https://html.spec.whatwg.org/multipage/#dom-source-src - make_getter!(Src, "src"); + make_url_getter!(Src, "src"); // https://html.spec.whatwg.org/multipage/#dom-source-src - make_setter!(SetSrc, "src"); + make_url_setter!(SetSrc, "src"); // https://html.spec.whatwg.org/multipage/#dom-source-type make_getter!(Type, "type"); @@ -131,10 +131,10 @@ impl HTMLSourceElementMethods for HTMLSourceElement { make_setter!(SetType, "type"); // https://html.spec.whatwg.org/multipage/#dom-source-srcset - make_getter!(Srcset, "srcset"); + make_url_getter!(Srcset, "srcset"); // https://html.spec.whatwg.org/multipage/#dom-source-srcset - make_setter!(SetSrcset, "srcset"); + make_url_setter!(SetSrcset, "srcset"); // https://html.spec.whatwg.org/multipage/#dom-source-sizes make_getter!(Sizes, "sizes"); diff --git a/components/script_bindings/webidls/HTMLSourceElement.webidl b/components/script_bindings/webidls/HTMLSourceElement.webidl index 92f75ff5995..542f4274741 100644 --- a/components/script_bindings/webidls/HTMLSourceElement.webidl +++ b/components/script_bindings/webidls/HTMLSourceElement.webidl @@ -8,11 +8,11 @@ interface HTMLSourceElement : HTMLElement { [HTMLConstructor] constructor(); [CEReactions] - attribute DOMString src; + attribute USVString src; [CEReactions] attribute DOMString type; [CEReactions] - attribute DOMString srcset; + attribute USVString srcset; [CEReactions] attribute DOMString sizes; [CEReactions] diff --git a/tests/wpt/meta/html/infrastructure/urls/dynamic-changes-to-base-urls/dynamic-urls.sub.html.ini b/tests/wpt/meta/html/infrastructure/urls/dynamic-changes-to-base-urls/dynamic-urls.sub.html.ini index e649c8fe2f0..37b9e7a9f82 100644 --- a/tests/wpt/meta/html/infrastructure/urls/dynamic-changes-to-base-urls/dynamic-urls.sub.html.ini +++ b/tests/wpt/meta/html/infrastructure/urls/dynamic-changes-to-base-urls/dynamic-urls.sub.html.ini @@ -8,8 +8,5 @@ [The 'src' attribute of the 'embed' element] expected: FAIL - [The 'src' attribute of the 'source' element] - expected: FAIL - [The 'data' attribute of the 'object' element] expected: FAIL diff --git a/tests/wpt/meta/html/infrastructure/urls/dynamic-changes-to-base-urls/historical.sub.xhtml.ini b/tests/wpt/meta/html/infrastructure/urls/dynamic-changes-to-base-urls/historical.sub.xhtml.ini index e2508bc182c..2bab2438435 100644 --- a/tests/wpt/meta/html/infrastructure/urls/dynamic-changes-to-base-urls/historical.sub.xhtml.ini +++ b/tests/wpt/meta/html/infrastructure/urls/dynamic-changes-to-base-urls/historical.sub.xhtml.ini @@ -8,8 +8,5 @@ [The 'src' attribute of the 'embed' element] expected: FAIL - [The 'src' attribute of the 'source' element] - expected: FAIL - [The 'data' attribute of the 'object' element] expected: FAIL diff --git a/tests/wpt/meta/html/semantics/embedded-content/media-elements/loading-the-media-resource/resource-selection-candidate-insert-before.html.ini b/tests/wpt/meta/html/semantics/embedded-content/media-elements/loading-the-media-resource/resource-selection-candidate-insert-before.html.ini deleted file mode 100644 index d90e5f052a2..00000000000 --- a/tests/wpt/meta/html/semantics/embedded-content/media-elements/loading-the-media-resource/resource-selection-candidate-insert-before.html.ini +++ /dev/null @@ -1,4 +0,0 @@ -[resource-selection-candidate-insert-before.html] - [inserting another source before the candidate] - expected: FAIL - diff --git a/tests/wpt/meta/html/semantics/embedded-content/media-elements/location-of-the-media-resource/currentSrc.html.ini b/tests/wpt/meta/html/semantics/embedded-content/media-elements/location-of-the-media-resource/currentSrc.html.ini deleted file mode 100644 index 2508434e7e1..00000000000 --- a/tests/wpt/meta/html/semantics/embedded-content/media-elements/location-of-the-media-resource/currentSrc.html.ini +++ /dev/null @@ -1,19 +0,0 @@ -[currentSrc.html] - [audio.currentSrc after adding source element with src attribute " "] - expected: FAIL - - [video.currentSrc after adding source element with src attribute "."] - expected: FAIL - - [video.currentSrc after adding source element with src attribute "data:,"] - expected: FAIL - - [audio.currentSrc after adding source element with src attribute "."] - expected: FAIL - - [audio.currentSrc after adding source element with src attribute "data:,"] - expected: FAIL - - [video.currentSrc after adding source element with src attribute " "] - expected: FAIL -