diff --git a/components/config/opts.rs b/components/config/opts.rs index 2bc90696a43..30634c76b9d 100644 --- a/components/config/opts.rs +++ b/components/config/opts.rs @@ -154,10 +154,6 @@ pub struct DebugOptions { /// Translate mouse input into touch events. pub convert_mouse_to_touch: bool, - /// Replace unpaires surrogates in DOM strings with U+FFFD. - /// See - pub replace_surrogates: bool, - /// Log GC passes and their durations. pub gc_profile: bool, @@ -184,7 +180,6 @@ impl DebugOptions { "gc-profile" => self.gc_profile = true, "profile-script-events" => self.profile_script_events = true, "relayout-event" => self.relayout_event = true, - "replace-surrogates" => self.replace_surrogates = true, "signpost" => self.signpost = true, "dump-style-stats" => self.dump_style_statistics = true, "trace-layout" => self.trace_layout = true, diff --git a/components/script/dom/characterdata.rs b/components/script/dom/characterdata.rs index 3c33a7c5010..f61deba4044 100644 --- a/components/script/dom/characterdata.rs +++ b/components/script/dom/characterdata.rs @@ -133,15 +133,10 @@ impl CharacterDataMethods for CharacterData { // https://dom.spec.whatwg.org/#dom-characterdata-substringdata fn SubstringData(&self, offset: u32, count: u32) -> Fallible { - let replace_surrogates = self - .upcast::() - .owner_doc() - .window() - .replace_surrogates(); let data = self.data.borrow(); // Step 1. let mut substring = String::new(); - let remaining = match split_at_utf16_code_unit_offset(&data, offset, replace_surrogates) { + let remaining = match split_at_utf16_code_unit_offset(&data, offset) { Ok((_, astral, s)) => { // As if we had split the UTF-16 surrogate pair in half // and then transcoded that to UTF-8 lossily, @@ -154,7 +149,7 @@ impl CharacterDataMethods for CharacterData { // Step 2. Err(()) => return Err(Error::IndexSize), }; - match split_at_utf16_code_unit_offset(remaining, count, replace_surrogates) { + match split_at_utf16_code_unit_offset(remaining, count) { // Steps 3. Err(()) => substring += remaining, // Steps 4. @@ -191,16 +186,11 @@ impl CharacterDataMethods for CharacterData { fn ReplaceData(&self, offset: u32, count: u32, arg: DOMString) -> ErrorResult { let mut new_data; { - let replace_surrogates = self - .upcast::() - .owner_doc() - .window() - .replace_surrogates(); let data = self.data.borrow(); let prefix; let replacement_before; let remaining; - match split_at_utf16_code_unit_offset(&data, offset, replace_surrogates) { + match split_at_utf16_code_unit_offset(&data, offset) { Ok((p, astral, r)) => { prefix = p; // As if we had split the UTF-16 surrogate pair in half @@ -214,7 +204,7 @@ impl CharacterDataMethods for CharacterData { }; let replacement_after; let suffix; - match split_at_utf16_code_unit_offset(remaining, count, replace_surrogates) { + match split_at_utf16_code_unit_offset(remaining, count) { // Steps 3. Err(()) => { replacement_after = ""; @@ -318,17 +308,7 @@ impl<'dom> LayoutCharacterDataHelpers<'dom> for LayoutDom<'dom, CharacterData> { /// The two string slices are such that: /// `before == s.to_utf16()[..offset - 1].to_utf8()` and /// `after == s.to_utf16()[offset + 1..].to_utf8()` -/// -/// # Panics -/// -/// Note that the third variant is only ever returned when the `-Z replace-surrogates` -/// command-line option is specified. -/// When it *would* be returned but the option is *not* specified, this function panics. -fn split_at_utf16_code_unit_offset( - s: &str, - offset: u32, - replace_surrogates: bool, -) -> Result<(&str, Option, &str), ()> { +fn split_at_utf16_code_unit_offset(s: &str, offset: u32) -> Result<(&str, Option, &str), ()> { let mut code_units = 0; for (i, c) in s.char_indices() { if code_units == offset { @@ -338,17 +318,9 @@ fn split_at_utf16_code_unit_offset( code_units += 1; if c > '\u{FFFF}' { if code_units == offset { - if replace_surrogates { - debug_assert_eq!(c.len_utf8(), 4); - return Ok((&s[..i], Some(c), &s[i + c.len_utf8()..])); - } - panic!( - "\n\n\ - Would split a surrogate pair in CharacterData API.\n\ - If you see this in real content, please comment with the URL\n\ - on https://github.com/servo/servo/issues/6873\n\ - \n" - ); + debug_assert_eq!(c.len_utf8(), 4); + warn!("Splitting a surrogate pair in CharacterData API."); + return Ok((&s[..i], Some(c), &s[i + c.len_utf8()..])); } code_units += 1; } diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index f496a1b8191..d55fc152eee 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -374,10 +374,6 @@ pub(crate) struct Window { /// won't be loaded. userscripts_path: Option, - /// Replace unpaired surrogates in DOM strings with U+FFFD. - /// See - replace_surrogates: bool, - /// Window's GL context from application #[ignore_malloc_size_of = "defined in script_thread"] #[no_trace] @@ -619,10 +615,6 @@ impl Window { self.userscripts_path.clone() } - pub(crate) fn replace_surrogates(&self) -> bool { - self.replace_surrogates - } - pub(crate) fn get_player_context(&self) -> WindowGLContext { self.player_context.clone() } @@ -2778,7 +2770,6 @@ impl Window { unminify_css: bool, local_script_source: Option, userscripts_path: Option, - replace_surrogates: bool, user_agent: Cow<'static, str>, player_context: WindowGLContext, #[cfg(feature = "webgpu")] gpu_id_hub: Arc, @@ -2864,7 +2855,6 @@ impl Window { prepare_for_screenshot, unminify_css, userscripts_path, - replace_surrogates, player_context, throttled: Cell::new(false), layout_marker: DomRefCell::new(Rc::new(Cell::new(true))), diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs index 9fe4b10e9b6..695b837e9ad 100644 --- a/components/script/script_thread.rs +++ b/components/script/script_thread.rs @@ -318,10 +318,6 @@ pub struct ScriptThread { /// won't be loaded userscripts_path: Option, - /// Replace unpaired surrogates in DOM strings with U+FFFD. - /// See - replace_surrogates: bool, - /// An optional string allowing the user agent to be set for testing. user_agent: Cow<'static, str>, @@ -957,7 +953,6 @@ impl ScriptThread { local_script_source: opts.local_script_source.clone(), unminify_css: opts.unminify_css, userscripts_path: opts.userscripts.clone(), - replace_surrogates: opts.debug.replace_surrogates, user_agent, player_context: state.player_context, node_ids: Default::default(), @@ -3146,7 +3141,6 @@ impl ScriptThread { self.unminify_css, self.local_script_source.clone(), self.userscripts_path.clone(), - self.replace_surrogates, self.user_agent.clone(), self.player_context.clone(), #[cfg(feature = "webgpu")] diff --git a/components/script_bindings/conversions.rs b/components/script_bindings/conversions.rs index ec05d0c33f8..e135acd2af5 100644 --- a/components/script_bindings/conversions.rs +++ b/components/script_bindings/conversions.rs @@ -17,7 +17,6 @@ use js::jsval::{ObjectValue, StringValue}; use js::rust::{ get_object_class, is_dom_class, maybe_wrap_value, HandleValue, MutableHandleValue, ToString, }; -use servo_config::opts; use crate::inheritance::Castable; use crate::reflector::Reflector; @@ -97,24 +96,8 @@ pub unsafe fn jsstring_to_str(cx: *mut JSContext, s: ptr::NonNull) -> match item { Ok(c) => s.push(c), Err(_) => { - // FIXME: Add more info like document URL in the message? - macro_rules! message { - () => { - "Found an unpaired surrogate in a DOM string. \ - If you see this in real web content, \ - please comment on https://github.com/servo/servo/issues/6564" - }; - } - if opts::get().debug.replace_surrogates { - error!(message!()); - s.push('\u{FFFD}'); - } else { - panic!(concat!( - message!(), - " Use `-Z replace-surrogates` \ - on the command line to make this non-fatal." - )); - } + error!("Found an unpaired surrogate in a DOM string."); + s.push('\u{FFFD}'); }, } } diff --git a/components/script_bindings/str.rs b/components/script_bindings/str.rs index d7968e74523..fcae1fd5abf 100644 --- a/components/script_bindings/str.rs +++ b/components/script_bindings/str.rs @@ -174,11 +174,9 @@ pub fn is_token(s: &[u8]) -> bool { /// unpaired surrogates. /// /// The hypothesis is that it does not matter much how exactly those values are -/// transformed, because passing unpaired surrogates into the DOM is very rare. -/// In order to test this hypothesis, Servo will panic when encountering any -/// unpaired surrogates on conversion to `DOMString` by default. (The command -/// line option `-Z replace-surrogates` instead causes Servo to replace the -/// unpaired surrogate by a U+FFFD replacement character.) +/// transformed, because passing unpaired surrogates into the DOM is very rare. +/// Instead Servo withh replace the unpaired surrogate by a U+FFFD replacement +/// character. /// /// Currently, the lack of crash reports about this issue provides some /// evidence to support the hypothesis. This evidence will hopefully be used to diff --git a/ports/servoshell/prefs.rs b/ports/servoshell/prefs.rs index 7826937c517..c3c13450dc7 100644 --- a/ports/servoshell/prefs.rs +++ b/ports/servoshell/prefs.rs @@ -679,7 +679,6 @@ fn print_debug_options_usage(app: &str) { "relayout-event", "Print notifications when there is a relayout.", ); - print_option("replace-surrogates", "Replace unpaires surrogates in DOM strings with U+FFFD. See https://github.com/servo/servo/issues/6564"); print_option( "show-fragment-borders", "Paint borders along fragment boundaries.", diff --git a/tests/wpt/meta/MANIFEST.json b/tests/wpt/meta/MANIFEST.json index 6f70312d634..ae655b6b0fe 100644 --- a/tests/wpt/meta/MANIFEST.json +++ b/tests/wpt/meta/MANIFEST.json @@ -505830,7 +505830,7 @@ [] ], "servodriver.py": [ - "4b82230bddddb56e7938d635feea55ba19c3d903", + "18035f9331e0d5df1c3fa916ce33a8866bea091a", [] ], "webkit.py": [ @@ -505892,7 +505892,7 @@ [] ], "executorservo.py": [ - "bc2021e1f5c05d55ce5a2a3fa35518efdd616415", + "e3369d24ebc6d6aac1b4632ef673a36602417745", [] ], "executorservodriver.py": [ diff --git a/tests/wpt/tests/tools/wptrunner/wptrunner/browsers/servodriver.py b/tests/wpt/tests/tools/wptrunner/wptrunner/browsers/servodriver.py index 4b82230bddd..18035f9331e 100644 --- a/tests/wpt/tests/tools/wptrunner/wptrunner/browsers/servodriver.py +++ b/tests/wpt/tests/tools/wptrunner/wptrunner/browsers/servodriver.py @@ -99,7 +99,6 @@ class ServoWebDriverBrowser(WebDriverBrowser): # For some reason rustls does not like the certificate generated by the WPT tooling. "--ignore-certificate-errors", "--window-size", "800x600", - "-Z", "replace-surrogates", "data:,", ] diff --git a/tests/wpt/tests/tools/wptrunner/wptrunner/executors/executorservo.py b/tests/wpt/tests/tools/wptrunner/wptrunner/executors/executorservo.py index bc2021e1f5c..e3369d24ebc 100644 --- a/tests/wpt/tests/tools/wptrunner/wptrunner/executors/executorservo.py +++ b/tests/wpt/tests/tools/wptrunner/wptrunner/executors/executorservo.py @@ -80,7 +80,7 @@ class ServoExecutor(ProcessTestExecutor): self.logger.error("Unable to find wpt-prefs.json") return default_path - def build_servo_command(self, test, extra_args=None, debug_opts="replace-surrogates"): + def build_servo_command(self, test, extra_args=None): args = [ "--hard-fail", "-u", "Servo/wptrunner", # See https://github.com/servo/servo/issues/30080. @@ -88,8 +88,6 @@ class ServoExecutor(ProcessTestExecutor): "--ignore-certificate-errors", "-z", self.test_url(test), ] - if debug_opts: - args += ["-Z", debug_opts] for stylesheet in self.browser.user_stylesheets: args += ["--user-stylesheet", stylesheet] for pref, value in self.environment.get('prefs', {}).items(): @@ -231,12 +229,11 @@ class ServoRefTestExecutor(ServoExecutor): extra_args = ["--exit", "--output=%s" % output_path, "--window-size", viewport_size or "800x600"] - debug_opts = "replace-surrogates" if dpi: extra_args += ["--device-pixel-ratio", dpi] - self.command = self.build_servo_command(test, extra_args, debug_opts) + self.command = self.build_servo_command(test, extra_args) if not self.interactive: self.proc = ProcessHandler(self.command,