From d917a1d406dcc9394cd93d32e7d0686b7cdec84b Mon Sep 17 00:00:00 2001 From: ldm0 Date: Tue, 8 Sep 2026 08:18:52 +0800 Subject: [PATCH] fix(url): preserve query and fragment delimiter semantics Use the existing url::quirks Web API getters and setters for URL and hyperlink query/fragment components, including Location.search. Empty components read as empty strings without removing href delimiters; setters strip only one leading delimiter so the remaining payload survives. Cover URL construction and parse results, connected and detached a/area elements in main, child and constructed documents, live URLSearchParams updates, and Location reads after document URL changes. Validation: cargo fmt --all; full workspace clippy with warnings denied; nextest 17597 passed, 13 skipped. Targeted tests 23/23 passed. CLI and CDP runs of 38 WPT cases both gain 55 passing subtests with no newly failing cases. CDP smoke remains 42/43; Puppeteer cannot start because node is missing. --- .../location_runtime/install.rs | 2 +- .../context_bootstrap/url_form/attributes.rs | 16 +- moli-renderer-v8/src/native_bridge/element.rs | 24 +-- moli-renderer-v8/src/script_vm/tests/mod.rs | 1 + .../src/script_vm/tests/url_components.rs | 139 ++++++++++++++++++ 5 files changed, 149 insertions(+), 33 deletions(-) create mode 100644 moli-renderer-v8/src/script_vm/tests/url_components.rs diff --git a/moli-renderer-v8/src/context_bootstrap/location_runtime/install.rs b/moli-renderer-v8/src/context_bootstrap/location_runtime/install.rs index 2324bf06d6..b574700469 100644 --- a/moli-renderer-v8/src/context_bootstrap/location_runtime/install.rs +++ b/moli-renderer-v8/src/context_bootstrap/location_runtime/install.rs @@ -459,7 +459,7 @@ fn location_attribute_getter<'s>( LocationAttribute::Search => { let search = url::Url::parse(¤t_href) .ok() - .and_then(|url| url.query().map(|query| format!("?{query}"))) + .map(|url| url::quirks::search(&url).to_owned()) .unwrap_or_default(); set_return_string(scope, rv, &search); } diff --git a/moli-renderer-v8/src/context_bootstrap/url_form/attributes.rs b/moli-renderer-v8/src/context_bootstrap/url_form/attributes.rs index 9aa94239aa..d0ed600163 100644 --- a/moli-renderer-v8/src/context_bootstrap/url_form/attributes.rs +++ b/moli-renderer-v8/src/context_bootstrap/url_form/attributes.rs @@ -255,7 +255,7 @@ fn url_attribute_getter<'s>( } UrlAttribute::Search => { let search = url_object_value(scope, this) - .and_then(|url| url.query().map(|query| format!("?{query}"))) + .map(|url| url::quirks::search(&url).to_owned()) .unwrap_or_default(); set_return_string(scope, rv, &search); } @@ -266,7 +266,7 @@ fn url_attribute_getter<'s>( } UrlAttribute::Hash => { let hash = url_object_value(scope, this) - .and_then(|url| url.fragment().map(|fragment| format!("#{fragment}"))) + .map(|url| url::quirks::hash(&url).to_owned()) .unwrap_or_default(); set_return_string(scope, rv, &hash); } @@ -444,11 +444,7 @@ fn url_writable_attribute_setter_callback<'s>( if let Some(mut url) = url_object_value(scope, this) && let Some(search) = url_attribute_usv_string(scope, args.get(0), attribute) { - if search.is_empty() { - url.set_query(None); - } else { - url.set_query(Some(search.trim_start_matches('?'))); - } + url::quirks::set_search(&mut url, &search); apply_url_update(scope, this, &url); } } @@ -456,11 +452,7 @@ fn url_writable_attribute_setter_callback<'s>( if let Some(mut url) = url_object_value(scope, this) && let Some(hash) = url_attribute_usv_string(scope, args.get(0), attribute) { - if hash.is_empty() { - url.set_fragment(None); - } else { - url.set_fragment(Some(hash.trim_start_matches('#'))); - } + url::quirks::set_hash(&mut url, &hash); apply_url_update(scope, this, &url); } } diff --git a/moli-renderer-v8/src/native_bridge/element.rs b/moli-renderer-v8/src/native_bridge/element.rs index 8111f2e2e4..bc092968af 100644 --- a/moli-renderer-v8/src/native_bridge/element.rs +++ b/moli-renderer-v8/src/native_bridge/element.rs @@ -2547,11 +2547,7 @@ fn anchor_search_getter_function<'s>( scope, args.this(), "", - |url| { - url.query() - .map(|query| format!("?{query}")) - .unwrap_or_default() - }, + |url| url::quirks::search(url).to_owned(), rv, ); } @@ -2572,11 +2568,7 @@ fn anchor_search_setter_function<'s>( let Some(value) = property_string_value(scope, args.get(0)) else { return; }; - if value.is_empty() { - url.set_query(None); - } else { - url.set_query(Some(value.trim_start_matches('?'))); - } + url::quirks::set_search(&mut url, &value); set_resolved_url_attribute(scope, runtime_ptr, handle, "href", &url); rv.set_undefined(); } @@ -2590,11 +2582,7 @@ fn anchor_hash_getter_function<'s>( scope, args.this(), "", - |url| { - url.fragment() - .map(|fragment| format!("#{fragment}")) - .unwrap_or_default() - }, + |url| url::quirks::hash(url).to_owned(), rv, ); } @@ -2615,11 +2603,7 @@ fn anchor_hash_setter_function<'s>( let Some(value) = property_string_value(scope, args.get(0)) else { return; }; - if value.is_empty() { - url.set_fragment(None); - } else { - url.set_fragment(Some(value.trim_start_matches('#'))); - } + url::quirks::set_hash(&mut url, &value); set_resolved_url_attribute(scope, runtime_ptr, handle, "href", &url); rv.set_undefined(); } diff --git a/moli-renderer-v8/src/script_vm/tests/mod.rs b/moli-renderer-v8/src/script_vm/tests/mod.rs index cea1db2c94..dc699f9cb0 100644 --- a/moli-renderer-v8/src/script_vm/tests/mod.rs +++ b/moli-renderer-v8/src/script_vm/tests/mod.rs @@ -15622,6 +15622,7 @@ mod queue_microtask; mod rendering_update; mod script_terminal_completion; mod streams; +mod url_components; mod webidl_collections; mod webidl_fetch; mod webidl_receivers; diff --git a/moli-renderer-v8/src/script_vm/tests/url_components.rs b/moli-renderer-v8/src/script_vm/tests/url_components.rs new file mode 100644 index 0000000000..a57561b55d --- /dev/null +++ b/moli-renderer-v8/src/script_vm/tests/url_components.rs @@ -0,0 +1,139 @@ +use super::*; + +fn assert_url_components(script: &str) { + let mut vm = new_parsed_test_vm( + "https://url-components.test/base/index.html", + "", + ); + let result = vm + .eval(&format!( + r#"(() => {{ +const assert = (condition, message) => {{ if (!condition) throw new Error(message); }}; +const frame = document.body.appendChild(document.createElement('iframe')); +const factories = [ + ['URL', href => new URL(href)], + ['URL.parse', href => URL.parse(href)], +]; +for (const [name, doc] of [ + ['main', document], + ['child', frame.contentDocument], + ['detached', document.implementation.createHTMLDocument('components')], +]) {{ + for (const tag of ['a', 'area']) {{ + for (const connected of [false, true]) {{ + factories.push([`${{name}}/${{tag}}/${{connected}}`, href => {{ + const node = doc.createElement(tag); + node.href = href; + if (connected) doc.body.appendChild(node); + return node; + }}]); + }} + }} +}} +{script} +return 'ok'; +}})()"# + )) + .expect("URL component semantics should match across exposed interfaces"); + assert_eq!(result, "ok"); +} + +#[test] +fn url_components_empty_getters_preserve_href() { + assert_url_components( + r#" +const cases = [ + ['', '', ''], ['?', '', ''], ['#', '', ''], ['?#', '', ''], + ['?q=%23#frag', '?q=%23', '#frag'], ['??', '??', ''], ['##', '', '##'], + ['?%3F#%23', '?%3F', '#%23'], ['?%20#%20', '?%20', '#%20'], +]; +for (const [name, create] of factories) { + for (const base of ['https://example.test/path', 'file:///tmp/item', 'mailto:user@example.test']) { + for (const [suffix, search, hash] of cases) { + const href = base + suffix; + const object = create(href); + assert(object.search === search, `${name}: search for ${href}`); + assert(object.hash === hash, `${name}: hash for ${href}`); + assert(object.href === href, `${name}: getters preserve href including empty delimiters`); + } + } +} +"#, + ); +} + +#[test] +fn url_components_setters_strip_only_one_delimiter() { + assert_url_components( + r#" +const base = 'https://example.test/path'; +for (const [name, create] of factories) { + for (const [property, delimiter] of [['search', '?'], ['hash', '#']]) { + for (const value of [delimiter.repeat(2), delimiter.repeat(3) + 'payload', 'payload', '%23%3F']) { + const object = create(base); + const expected = value.startsWith(delimiter) ? value : delimiter + value; + object[property] = value; + assert(object[property] === expected, `${name}: ${property} retains payload in ${value}`); + assert(object.href === base + expected, `${name}: setter preserves remaining delimiters`); + } + } + const object = create(base + '#keep'); + const params = object.searchParams; + object.search = '??q=value'; + assert(object.href === base + '??q=value#keep', `${name}: query update retains fragment`); + if (params) { + assert(object.searchParams === params && params.get('?q') === 'value', 'live URLSearchParams observes the unstripped query payload'); + params.set('?q', 'next'); + assert(object.search === '?%3Fq=next' && object.hash === '#keep', 'URLSearchParams encodes the literal query marker'); + } +} +"#, + ); +} + +#[test] +fn url_components_clearing_distinguishes_empty_from_absent() { + assert_url_components( + r#" +const base = 'https://example.test/path'; +for (const [name, create] of factories) { + const object = create(base + '?q=value#frag'); + const params = object.searchParams; + object.search = '?'; + assert(object.search === '' && object.href === base + '?#frag', `${name}: empty query preserves its delimiter`); + if (params) assert(params.size === 0, 'empty query clears live URLSearchParams'); + object.hash = '#'; + assert(object.hash === '' && object.href === base + '?#', `${name}: empty fragment preserves its delimiter`); + object.search = ''; + assert(object.search === '' && object.href === base + '#', `${name}: clearing query preserves empty fragment`); + object.hash = ''; + assert(object.hash === '' && object.href === base, `${name}: clearing fragment removes its delimiter`); +} +"#, + ); +} + +#[test] +fn location_components_empty_getters_preserve_document_urls() { + assert_url_components( + r#" +const cases = [ + ['', '', ''], ['?', '', ''], ['#', '', ''], ['?#', '', ''], + ['??query##fragment', '??query', '##fragment'], ['?%3F#%23', '?%3F', '#%23'], +]; +for (const [suffix, search, hash] of cases) { + const href = 'https://url-components.test/base/index.html' + suffix; + history.replaceState(null, '', href); + assert(location.search === search && location.hash === hash, `main Location components for ${suffix}`); + assert(location.href === href && document.URL === href, 'Location getters preserve the document URL'); + const child = document.createElement('iframe'); + child.src = 'about:blank' + suffix; + document.body.appendChild(child); + const childLocation = child.contentWindow.location; + assert(childLocation.search === search && childLocation.hash === hash, `child Location components for ${suffix}`); + assert(childLocation.href === 'about:blank' + suffix, 'child Location getters preserve delimiters'); + child.remove(); +} +"#, + ); +}