diff options
11 files changed, 424 insertions, 12 deletions
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<Root<Element>> { - self.get_element_by_id(&Atom::from(fragid)).or_else(|| { - let check_anchor = |node: &HTMLAnchorElement| { - let elem = node.upcast::<Element>(); - elem.get_attribute(&ns!(), &atom!("name")) - .map_or(false, |attr| &**attr.value() == fragid) - }; - let doc_node = self.upcast::<Node>(); - 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<Root<Element>> { + let check_anchor = |node: &HTMLAnchorElement| { + let elem = node.upcast::<Element>(); + elem.get_attribute(&ns!(), &atom!("name")) + .map_or(false, |attr| &**attr.value() == name) + }; + let doc_node = self.upcast::<Node>(); + doc_node.traverse_preorder() + .filter_map(Root::downcast) + .find(|node| check_anchor(&node)) + .map(Root::upcast) } pub fn hit_test(&self, page_point: &Point2D<f32>) -> Option<UntrustedNodeAddress> { 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 @@ +<!DOCTYPE html> +<title>Navigate to Fragment - issue #1716</title> +<meta name=timeout content=long> +<script src="/resources/testharness.js"></script> +<script src="/resources/testharnessreport.js"></script> +<style> + .test-target{ + height:20em; + } + .bottom-padding{ + height:200em; + } +</style> +<p id="test0"> +Manual test to check each link goes to the right place.</p> +<table id="tests-table"> + <thead> + <th>Fragment Identifier</th> + <th>Intended Fragment</th> + <th>Test Target</th> + <th>Description</th> + </thead> + <tbody> + <tr> + <td><a href="#top">#top</a></td> + <td>Document</td> + <td>test0</td> + <td>Top of document</td> + </tr> + <tr> + <td><a href="#TOP">#TOP</a></a></td> + <td>Document</td> + <td>test0</td> + <td>Top of document</td> + </tr> + <tr> + <td><a href="#Top">#Top</a></a></td> + <td><p id="Top"></td> + <td>test8</td> + <td>Id takes precendence over "top"</td> + </tr> + <tr> + <td><a href="#sanity-check">#sanity-check</a></td> + <td><p id="sanity-check"></td> + <td>test1</td> + <td>Sanity check</td> + </tr> + <tr> + <td><a href="#has%20space">#has%20space</a></td> + <td><p id="has space"></td> + <td>test2</td> + <td>Contains a space</td> + </tr> + <tr> + <td><a href="#escaped%20space">#escaped%20space</a></td> + <td><p id="escaped%20space"></td> + <td></td> + <td>Contains an escaped space. Only decoded fragid is used for ids.</td> + </tr> + <tr> + <td><a href="#escaped%20unescaped%20collide">#escaped%20unescaped%20collide</a></td> + <td><p id="escaped unescaped collide"></td> + <td>test4</td> + <td>Another element has the same id but pecent-encoded. The decoded one should win.</td> + </tr> + <tr> + <td><a href="#name-match">#name-match</a></td> + <td><a name="name-match"></td> + <td>test5</td> + <td></td> + </tr> + <tr> + <td><a href="#name-collide">#name-collide</a></td> + <td><a id="name-collide"></td> + <td>test6</td> + <td>Same id as an anchor name. Id should win.</td> + </tr> + <tr> + <td><a href="#escaped%20name">#escaped%20name</a></td> + <td><a name="escaped%20name"></td> + <td>test7</td> + <td>Undecoded fragid should be used for anchor names.</td> + </tr> + </tbody> +</table> + +<div class="test-target" id="test1"> + <p id="sanity-check">span id="sanity-check"</p><p>SUCCESS test1</p> +</div> +<div class="test-target" id="test2"> + <p id="has space">span id="has space"</p><p>SUCCESS test2</p> +</div> +<div class="test-target" id="test3"> + <p id="escaped%20space">span id="escaped%20space"</p><p>FAIL test3</p> + Not in whatwg spec, but a <em>tolerant</em> implementation would do this to give content creator what they probably intended. +</div> +<div class="test-target" id="test4"> + <p id="escaped unescaped collide">span id="escaped unescaped collide"</p><p>SUCCESS test4</p> +</div> +<div class="test-target"> + <p id="escaped%20unescaped%20collide">span id="escaped%20unescaped%20collide"</p><p>FAIL test4</p> + Not in whatwg spec, but a <em>tolerant</em> implementation would do this to give content creator what they probably intended. +</div> +<div class="test-target" id="test5"> + <a name="name-match">a name="name-match"</a><p>SUCCESS test5</p> +</div> +<div class="test-target"> + <a name="name-collide">a name="name-collide"</a><p>FAIL test6</p> + An anchor name and an id have the same value. The id should take precence! +</div> +<div class="test-target" id="test6"> + <a id="name-collide">a id="name-collide"</a><p>SUCCESS test6</p> +</div> +<div class="test-target" id="test7"> + <a name="escaped%20name">a name="escaped%20name"</a><p>SUCCESS test7</p> +</div> +<div class="test-target" id="test8"> + <p id="Top">p id="Top"</a><p>SUCCESS test8</p> +</div> +<div class="bottom-padding"/> 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 @@ +<!doctype html> +<title>Fragment Navigation: fragment id should be percent-decoded</title> +<meta name=timeout content=long> +<script src="/resources/testharness.js"></script> +<script src="/resources/testharnessreport.js"></script> +<body> +<div></div> +<div id="has two spaces" style="position:absolute; top:200px;"></div> +<div id="escape%20collision" style="position:absolute; top:300px;"></div> +<div id="escape collision" style="position:absolute; top:400px;"></div> +<div id="do%20not%20go%20here" style="position:absolute; top:400px;"></div> +<div style="height:200em;"></div> +<script> +var steps = [{ + fragid:'has%20two%20spaces', + handler: function(){ + assert_equals( scrollPosition(), 200 ); + } + },{ + fragid:'escape%20collision', + handler: function(){ + assert_equals( scrollPosition(), 400 ); + } + },{ + fragid:'do%20not%20go%20here', + handler: function(){ + // don't move + assert_equals( scrollPosition(), 400 ); + } + }]; + +function scrollPosition(){ + return document.documentElement.scrollTop || document.body.scrollTop; +} + +function runNextStep(){ + if( steps.length > 0 ) { + var step = steps.shift(); + var listener = t.step_func( function(){ + step.handler(); + runNextStep(); + }); + scrollToFragmentThenDo( step.fragid, listener ); + } else { + t.done(); + } +} + +function scrollToFragmentThenDo( fragid, then ){ + location.hash = fragid; + setTimeout( then, 1 ); +} + +var t = async_test(); +t.step( function(){ + assert_equals(location.hash, "", "Page must be loaded with no hash"); + runNextStep(); +}) +</script> 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 @@ +<!doctype html> +<title>Fragment Navigation: scroll to anchor name is lower priority than equal id</title> +<meta name=timeout content=long> +<script src="/resources/testharness.js"></script> +<script src="/resources/testharnessreport.js"></script> +<body> +<div></div> +<a name="anchor1" style="position:absolute; top:200px;"></a> +<div id="id-equals-anchor" style="position:absolute; top:300px;"></div> +<a name="id-equals-anchor" style="position:absolute; top:400px;"></a> +<div style="height:200em;"></div> +<script> +var steps = [{ + fragid:'anchor1', + handler: function(){ + assert_equals( scrollPosition(), 200 ); + } + },{ + fragid:'id-equals-anchor', + handler: function(){ + // id still takes precedence over anchor name + assert_equals( scrollPosition(), 300 ); + } + }]; + +function scrollPosition(){ + return document.documentElement.scrollTop || document.body.scrollTop; +} + +function runNextStep(){ + if( steps.length > 0 ) { + var step = steps.shift(); + var listener = t.step_func( function(){ + step.handler(); + runNextStep(); + }); + scrollToFragmentThenDo( step.fragid, listener ); + } else { + t.done(); + } +} + +function scrollToFragmentThenDo( fragid, then ){ + location.hash = fragid; + setTimeout( then, 1 ); +} + +var t = async_test(); +t.step( function(){ + assert_equals(location.hash, "", "Page must be loaded with no hash"); + runNextStep(); +}) +</script> 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 @@ +<!doctype html> +<title>Fragment Navigation: TOP is a valid element id, which overrides navigating to top of the document</title> +<meta name=timeout content=long> +<script src="/resources/testharness.js"></script> +<script src="/resources/testharnessreport.js"></script> +<body> +<div></div> +<div id="Top" style="position:absolute; top:200px;"></div> +<div style="height:200em; position:relative;"></div> +<script> +var steps = [{ + fragid:'Top', + handler: function(){ + assert_equals( scrollPosition(), 200 ); + } + },{ + // scroling to top should work when fragid differs from id by case. + fragid:'top', + handler: function(){ + assert_equals( scrollPosition(), 0 ); + } + }]; + +function scrollPosition(){ + return document.documentElement.scrollTop || document.body.scrollTop; +} + +function runNextStep(){ + if( steps.length > 0 ) { + var step = steps.shift(); + var listener = t.step_func( function(){ + step.handler(); + runNextStep(); + }); + scrollToFragmentThenDo( step.fragid, listener ); + } else { + t.done(); + } +} + +function scrollToFragmentThenDo( fragid, then ){ + location.hash = fragid; + setTimeout( then, 1 ); +} + +var t = async_test(); +t.step( function(){ + assert_equals(location.hash, "", "Page must be loaded with no hash"); + runNextStep(); +}) +</script> 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 @@ +<!doctype html> +<title>Fragment Navigation: When fragid is TOP scroll to the top of the document</title> +<meta name=timeout content=long> +<script src="/resources/testharness.js"></script> +<script src="/resources/testharnessreport.js"></script> +<body> +<div></div> +<div id="not-the-top"></div> +<div style="height:200em"></div> +<script> +var steps = [{ + fragid:'not-the-top', + handler: function(){ + assert_not_equals( scrollPosition(), 0 ); + } + },{ + fragid:'top', + handler: function(){ + assert_equals( scrollPosition(), 0 ); + } + },{ + fragid:'not-the-top', + handler: function(){ + assert_not_equals( scrollPosition(), 0 ); + } + },{ + fragid:'TOP', + handler: function(){ + assert_equals( scrollPosition(), 0 ); + } + }]; + +function scrollPosition(){ + return document.documentElement.scrollTop || document.body.scrollTop; +} + +function runNextStep(){ + if( steps.length > 0 ) { + var step = steps.shift(); + var listener = t.step_func( function(){ + step.handler(); + runNextStep(); + }); + scrollToFragmentThenDo( step.fragid, listener ); + } else { + t.done(); + } +} + +function scrollToFragmentThenDo( fragid, then ){ + location.hash = fragid; + setTimeout( then, 1 ); +} + +var t = async_test(); +t.step( function(){ + assert_equals(location.hash, "", "Page must be loaded with no hash"); + runNextStep(); +}) +</script> |