script: Fix the scroll to top behavior

When finding the indicated fragment, do not use the document element to indicate
the top of the Document, and when scrolling to the frament and we do not find a
element, scrolling the top if the fragment is empty or equal to "top".

components/script/dom/document.rs

@@ -580,57 +580,46 @@ impl Document {
     /// https://html.spec.whatwg.org/multipage/#the-indicated-part-of-the-document
     pub fn find_fragment_node(&self, fragid: &str) -> Option<Root<Element>> {
         // 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);
 
         // 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::<Node>().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::<Node>().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

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",

tests/wpt/mozilla/tests/mozilla/scroll_top_null_target.html

@@ -0,0 +1,21 @@
+<!doctype html>
+<html>
+    <head>
+        <meta charset="utf-8">
+        <title></title>
+        <script src="/resources/testharness.js"></script>
+        <script src="/resources/testharnessreport.js"></script>
+    </head>
+    <body>
+        <a id="test"></a>
+    </body>
+    <script>
+     test(function() {
+         location.hash = "test";
+         assert_equals(document.querySelector(":target"), document.getElementById("test"),
+                       "Target shoud be the same with the test anchor!");
+         location.hash = "";
+         assert_equals(document.querySelector(":target"), null, "Target should be null!");
+     });
+    </script>
+</html>