diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index 0bf77ccf73dc..b4273233c37b 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -580,26 +580,18 @@ impl Document { /// https://html.spec.whatwg.org/multipage/#the-indicated-part-of-the-document pub fn find_fragment_node(&self, fragid: &str) -> Option> { // Step 1 is not handled here; the fragid is already obtained by the calling function - // Step 2 - if fragid.is_empty() { - self.GetDocumentElement() - } else { - // Step 3 & 4 - percent_decode(fragid.as_bytes()).decode_utf8().ok() - // Step 5 - .and_then(|decoded_fragid| self.get_element_by_id(&Atom::from(decoded_fragid))) - // Step 6 - .or_else(|| self.get_anchor_by_name(fragid)) - // Step 7 - .or_else(|| if fragid.eq_ignore_ascii_case("top") { - self.GetDocumentElement() - } else { - // Step 8 - None - }) - } + // Step 2: Simply use None to indicate the top of the document. + // Step 3 & 4 + percent_decode(fragid.as_bytes()).decode_utf8().ok() + // Step 5 + .and_then(|decoded_fragid| self.get_element_by_id(&Atom::from(decoded_fragid))) + // Step 6 + .or_else(|| self.get_anchor_by_name(fragid)) + // Step 7 & 8 } + /// Scroll to the target element, and when we do not find a target + /// and the fragment is empty or "top", scroll to the top. /// https://html.spec.whatwg.org/multipage/#scroll-to-the-fragment-identifier pub fn check_and_scroll_fragment(&self, fragment: &str) { let target = self.find_fragment_node(fragment); @@ -607,30 +599,27 @@ impl Document { // Step 1 self.set_target_element(target.r()); - let point = if fragment.is_empty() || fragment.eq_ignore_ascii_case("top") { + let point = target.r().map(|element| { + // FIXME(#8275, pcwalton): This is pretty bogus when multiple layers are involved. + // Really what needs to happen is that this needs to go through layout to ask which + // layer the element belongs to, and have it send the scroll message to the + // compositor. + let rect = element.upcast::().bounding_content_box(); + + // In order to align with element edges, we snap to unscaled pixel boundaries, since + // the paint thread currently does the same for drawing elements. This is important + // for pages that require pixel perfect scroll positioning for proper display + // (like Acid2). Since we don't have the device pixel ratio here, this might not be + // accurate, but should work as long as the ratio is a whole number. Once #8275 is + // fixed this should actually take into account the real device pixel ratio. + (rect.origin.x.to_nearest_px() as f32, rect.origin.y.to_nearest_px() as f32) + }).or_else(|| if fragment.is_empty() || fragment.eq_ignore_ascii_case("top") { // FIXME(stshine): this should be the origin of the stacking context space, // which may differ under the influence of writing mode. Some((0.0, 0.0)) } else { - target.r().map(|element| { - // FIXME(#8275, pcwalton): This is pretty bogus when multiple layers - // are involved. Really what needs to happen is that this needs to go - // through layout to ask which layer the element belongs to, and have - // it send the scroll message to the compositor. - let rect = element.upcast::().bounding_content_box(); - - // In order to align with element edges, we snap to unscaled pixel - // boundaries, since the paint thread currently does the same for - // drawing elements. This is important for pages that require pixel - // perfect scroll positioning for proper display (like Acid2). Since - // we don't have the device pixel ratio here, this might not be - // accurate, but should work as long as the ratio is a whole number. - // Once #8275 is fixed this should actually take into account the - // real device pixel ratio. - (rect.origin.x.to_nearest_px() as f32, - rect.origin.y.to_nearest_px() as f32) - }) - }; + None + }); if let Some((x, y)) = point { // Step 3 diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index a1b285c21bab..64fcd7c0e98d 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -14690,6 +14690,12 @@ "url": "/_mozilla/mozilla/scrollTo.html" } ], + "mozilla/scroll_top_null_target.html": [ + { + "path": "mozilla/scroll_top_null_target.html", + "url": "/_mozilla/mozilla/scroll_top_null_target.html" + } + ], "mozilla/send-arraybuffer.htm": [ { "path": "mozilla/send-arraybuffer.htm", diff --git a/tests/wpt/mozilla/tests/mozilla/scroll_top_null_target.html b/tests/wpt/mozilla/tests/mozilla/scroll_top_null_target.html new file mode 100644 index 000000000000..512bf34d506a --- /dev/null +++ b/tests/wpt/mozilla/tests/mozilla/scroll_top_null_target.html @@ -0,0 +1,21 @@ + + + + + + + + + + + + +