diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs index a3af57b0814..de1172ed6b0 100644 --- a/components/script/dom/document.rs +++ b/components/script/dom/document.rs @@ -107,6 +107,7 @@ use style::context::ReflowGoal; use style::restyle_hints::ElementSnapshot; use style::servo::Stylesheet; use time; +use url::percent_encoding::percent_decode; use url::{Host, Url}; use util::str::{DOMString, split_html_space_chars, str_join}; @@ -515,18 +516,38 @@ impl Document { /// Attempt to find a named element in this page's document. /// https://html.spec.whatwg.org/multipage/#the-indicated-part-of-the-document pub fn find_fragment_node(&self, fragid: &str) -> Option> { - self.get_element_by_id(&Atom::from(fragid)).or_else(|| { - let check_anchor = |node: &HTMLAnchorElement| { - let elem = node.upcast::(); - elem.get_attribute(&ns!(), &atom!("name")) - .map_or(false, |attr| &**attr.value() == fragid) - }; - let doc_node = self.upcast::(); - doc_node.traverse_preorder() - .filter_map(Root::downcast) - .find(|node| check_anchor(&node)) - .map(Root::upcast) - }) + // 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 + String::from_utf8(percent_decode(fragid.as_bytes())).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.to_lowercase() == "top" { + self.GetDocumentElement() + } else { + // Step 8 + None + }) + } + } + + fn get_anchor_by_name(&self, name: &str) -> Option> { + let check_anchor = |node: &HTMLAnchorElement| { + let elem = node.upcast::(); + elem.get_attribute(&ns!(), &atom!("name")) + .map_or(false, |attr| &**attr.value() == name) + }; + let doc_node = self.upcast::(); + doc_node.traverse_preorder() + .filter_map(Root::downcast) + .find(|node| check_anchor(&node)) + .map(Root::upcast) } pub fn hit_test(&self, page_point: &Point2D) -> Option { diff --git a/tests/html/navigate-to-fragid.html b/tests/html/navigate-to-fragid.html new file mode 100644 index 00000000000..0c5fce90339 --- /dev/null +++ b/tests/html/navigate-to-fragid.html @@ -0,0 +1,120 @@ + +Navigate to Fragment - issue #1716 + + + + +

+Manual test to check each link goes to the right place.

+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
Fragment IdentifierIntended FragmentTest TargetDescription
#topDocumenttest0Top of document
#TOPDocumenttest0Top of document
#Top<p id="Top">test8Id takes precendence over "top"
#sanity-check<p id="sanity-check">test1Sanity check
#has%20space<p id="has space">test2Contains a space
#escaped%20space<p id="escaped%20space">Contains an escaped space. Only decoded fragid is used for ids.
#escaped%20unescaped%20collide<p id="escaped unescaped collide">test4Another element has the same id but pecent-encoded. The decoded one should win.
#name-match<a name="name-match">test5
#name-collide<a id="name-collide">test6Same id as an anchor name. Id should win.
#escaped%20name<a name="escaped%20name">test7Undecoded fragid should be used for anchor names.
+ +
+

span id="sanity-check"

SUCCESS test1

+
+
+

span id="has space"

SUCCESS test2

+
+
+

span id="escaped%20space"

FAIL test3

+ Not in whatwg spec, but a tolerant implementation would do this to give content creator what they probably intended. +
+
+

span id="escaped unescaped collide"

SUCCESS test4

+
+
+

span id="escaped%20unescaped%20collide"

FAIL test4

+ Not in whatwg spec, but a tolerant implementation would do this to give content creator what they probably intended. +
+
+ a name="name-match"

SUCCESS test5

+
+
+ a name="name-collide"

FAIL test6

+ An anchor name and an id have the same value. The id should take precence! +
+
+ a id="name-collide"

SUCCESS test6

+
+
+ a name="escaped%20name"

SUCCESS test7

+
+
+

p id="Top"

SUCCESS test8

+
+
diff --git a/tests/wpt/metadata/MANIFEST.json b/tests/wpt/metadata/MANIFEST.json index b1c8a4f322a..6f6feaca966 100644 --- a/tests/wpt/metadata/MANIFEST.json +++ b/tests/wpt/metadata/MANIFEST.json @@ -33813,6 +33813,34 @@ "path": "WebIDL/ecmascript-binding/has-instance.html", "url": "/WebIDL/ecmascript-binding/has-instance.html" } + ], + "html/browsers/browsing-the-web/scroll-to-fragid/scroll-frag-percent-encoded.html": [ + { + "path": "html/browsers/browsing-the-web/scroll-to-fragid/scroll-frag-percent-encoded.html", + "timeout": "long", + "url": "/html/browsers/browsing-the-web/scroll-to-fragid/scroll-frag-percent-encoded.html" + } + ], + "html/browsers/browsing-the-web/scroll-to-fragid/scroll-to-anchor-name.html": [ + { + "path": "html/browsers/browsing-the-web/scroll-to-fragid/scroll-to-anchor-name.html", + "timeout": "long", + "url": "/html/browsers/browsing-the-web/scroll-to-fragid/scroll-to-anchor-name.html" + } + ], + "html/browsers/browsing-the-web/scroll-to-fragid/scroll-to-id-top.html": [ + { + "path": "html/browsers/browsing-the-web/scroll-to-fragid/scroll-to-id-top.html", + "timeout": "long", + "url": "/html/browsers/browsing-the-web/scroll-to-fragid/scroll-to-id-top.html" + } + ], + "html/browsers/browsing-the-web/scroll-to-fragid/scroll-to-top.html": [ + { + "path": "html/browsers/browsing-the-web/scroll-to-fragid/scroll-to-top.html", + "timeout": "long", + "url": "/html/browsers/browsing-the-web/scroll-to-fragid/scroll-to-top.html" + } ] } }, diff --git a/tests/wpt/metadata/html/browsers/browsing-the-web/scroll-to-fragid/scroll-frag-percent-encoded.html.ini b/tests/wpt/metadata/html/browsers/browsing-the-web/scroll-to-fragid/scroll-frag-percent-encoded.html.ini new file mode 100644 index 00000000000..5df3f6d56c9 --- /dev/null +++ b/tests/wpt/metadata/html/browsers/browsing-the-web/scroll-to-fragid/scroll-frag-percent-encoded.html.ini @@ -0,0 +1,5 @@ +[scroll-frag-percent-encoded.html] + type: testharness + + [Fragment Navigation: fragment id should be percent-decoded] + expected: FAIL diff --git a/tests/wpt/metadata/html/browsers/browsing-the-web/scroll-to-fragid/scroll-to-anchor-name.html.ini b/tests/wpt/metadata/html/browsers/browsing-the-web/scroll-to-fragid/scroll-to-anchor-name.html.ini new file mode 100644 index 00000000000..5b635ebeb40 --- /dev/null +++ b/tests/wpt/metadata/html/browsers/browsing-the-web/scroll-to-fragid/scroll-to-anchor-name.html.ini @@ -0,0 +1,5 @@ +[scroll-to-anchor-name.html] + type: testharness + + [Fragment Navigation: scroll to anchor name is lower priority than equal id] + expected: FAIL diff --git a/tests/wpt/metadata/html/browsers/browsing-the-web/scroll-to-fragid/scroll-to-id-top.html.ini b/tests/wpt/metadata/html/browsers/browsing-the-web/scroll-to-fragid/scroll-to-id-top.html.ini new file mode 100644 index 00000000000..ec9fd8ae554 --- /dev/null +++ b/tests/wpt/metadata/html/browsers/browsing-the-web/scroll-to-fragid/scroll-to-id-top.html.ini @@ -0,0 +1,5 @@ +[scroll-to-id-top.html] + type: testharness + + [Fragment Navigation: TOP is a valid element id, which overrides navigating to top of the document] + expected: FAIL diff --git a/tests/wpt/metadata/html/browsers/browsing-the-web/scroll-to-fragid/scroll-to-top.html.ini b/tests/wpt/metadata/html/browsers/browsing-the-web/scroll-to-fragid/scroll-to-top.html.ini new file mode 100644 index 00000000000..7af30787e60 --- /dev/null +++ b/tests/wpt/metadata/html/browsers/browsing-the-web/scroll-to-fragid/scroll-to-top.html.ini @@ -0,0 +1,5 @@ +[scroll-to-top.html] + type: testharness + + [Fragment Navigation: When fragid is TOP scroll to the top of the document] + expected: FAIL diff --git a/tests/wpt/web-platform-tests/html/browsers/browsing-the-web/scroll-to-fragid/scroll-frag-percent-encoded.html b/tests/wpt/web-platform-tests/html/browsers/browsing-the-web/scroll-to-fragid/scroll-frag-percent-encoded.html new file mode 100644 index 00000000000..3196d8be8bb --- /dev/null +++ b/tests/wpt/web-platform-tests/html/browsers/browsing-the-web/scroll-to-fragid/scroll-frag-percent-encoded.html @@ -0,0 +1,59 @@ + +Fragment Navigation: fragment id should be percent-decoded + + + + +
+
+
+
+
+
+ diff --git a/tests/wpt/web-platform-tests/html/browsers/browsing-the-web/scroll-to-fragid/scroll-to-anchor-name.html b/tests/wpt/web-platform-tests/html/browsers/browsing-the-web/scroll-to-fragid/scroll-to-anchor-name.html new file mode 100644 index 00000000000..43dbaf9e297 --- /dev/null +++ b/tests/wpt/web-platform-tests/html/browsers/browsing-the-web/scroll-to-fragid/scroll-to-anchor-name.html @@ -0,0 +1,53 @@ + +Fragment Navigation: scroll to anchor name is lower priority than equal id + + + + +
+ +
+ +
+ diff --git a/tests/wpt/web-platform-tests/html/browsers/browsing-the-web/scroll-to-fragid/scroll-to-id-top.html b/tests/wpt/web-platform-tests/html/browsers/browsing-the-web/scroll-to-fragid/scroll-to-id-top.html new file mode 100644 index 00000000000..601d40a2a1b --- /dev/null +++ b/tests/wpt/web-platform-tests/html/browsers/browsing-the-web/scroll-to-fragid/scroll-to-id-top.html @@ -0,0 +1,51 @@ + +Fragment Navigation: TOP is a valid element id, which overrides navigating to top of the document + + + + +
+
+
+ diff --git a/tests/wpt/web-platform-tests/html/browsers/browsing-the-web/scroll-to-fragid/scroll-to-top.html b/tests/wpt/web-platform-tests/html/browsers/browsing-the-web/scroll-to-fragid/scroll-to-top.html new file mode 100644 index 00000000000..3265a71bf7c --- /dev/null +++ b/tests/wpt/web-platform-tests/html/browsers/browsing-the-web/scroll-to-fragid/scroll-to-top.html @@ -0,0 +1,60 @@ + +Fragment Navigation: When fragid is TOP scroll to the top of the document + + + + +
+
+
+