Bug 1595185 [wpt PR 20184] - Add more scroll to text WPTs, a=testonly
authorNick Burris <nburris@chromium.org>
Mon, 25 Nov 2019 19:17:24 +0000
changeset 504539 8c4ffe8d76ac8bf9b1dfeacdf4a7ef22c77a2d84
parent 504538 ef211f079e9c04e0435c57a82816036791520047
child 504540 42a16ff896754623ecaa7a41bb320c70c16c5afd
push id101897
push userwptsync@mozilla.com
push dateFri, 29 Nov 2019 11:10:32 +0000
treeherderautoland@47be1b3fdda6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstestonly
bugs1595185, 20184, 1903624, 714381
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1595185 [wpt PR 20184] - Add more scroll to text WPTs, a=testonly Automatic update from web-platform-tests Add more scroll to text WPTs Adds more web platform tests for scroll to text: - Combination of non-text directive and text directive - Scroll second text directive into view if first doesn't match - Matching shadow DOM - Don't scroll to hidden text - Text that requires a horizontal scroll into view - Also fixed non-ASCII test case (and changed to Japanese example) Change-Id: Id7e21a185897267dc565b19adc7bfdfb5fe70974 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1903624 Commit-Queue: Nick Burris <nburris@chromium.org> Reviewed-by: David Bokan <bokan@chromium.org> Cr-Commit-Position: refs/heads/master@{#714381} -- wpt-commits: fb6ccd56fa8c857991e49c59fdde160fabfe8507 wpt-pr: 20184
testing/web-platform/tests/scroll-to-text-fragment/scroll-to-text-fragment-target.html
testing/web-platform/tests/scroll-to-text-fragment/scroll-to-text-fragment.html
--- a/testing/web-platform/tests/scroll-to-text-fragment/scroll-to-text-fragment-target.html
+++ b/testing/web-platform/tests/scroll-to-text-fragment/scroll-to-text-fragment-target.html
@@ -1,14 +1,15 @@
 <!doctype html>
 <title>Navigating to a text fragment anchor</title>
 <script>
 function isInView(element) {
   let rect = element.getBoundingClientRect();
-  return rect.top >= 0 && rect.top <= window.innerHeight;
+  return rect.top >= 0 && rect.top <= window.innerHeight
+      && rect.left >= 0 && rect.left <= window.innerWidth;
 }
 
 function checkScroll() {
   let bc = new BroadcastChannel('scroll-to-text-fragment');
 
   let position = 'unknown';
   if (window.scrollY == 0)
     position = 'top';
@@ -17,52 +18,56 @@ function checkScroll() {
   else if (isInView(document.getElementById('text')))
     position = 'text';
   else if (isInView(document.getElementById('more-text')))
     position = 'more-text';
   else if (isInView(document.getElementById('cross-node-context')))
     position = 'cross-node-context';
   else if (isInView(document.getElementById('text-directive-parameters')))
     position = 'text-directive-parameters';
+  else if (isInView(document.getElementById('shadow-parent')))
+    position = 'shadow-parent';
+  else if (isInView(document.getElementById('hidden')))
+    position = 'hidden';
+  else if (isInView(document.getElementById('horizontal-scroll')) && window.scrollX > 0)
+    position = 'horizontal-scroll';
 
   bc.postMessage({ scrollPosition: position, href: window.location.href });
   bc.close();
   window.close();
 }
 </script>
 <style>
-  body {
-    height: 6200px;
-  }
-  #element {
-    position: absolute;
-    top: 2000px;
-  }
-  #text {
-    position: absolute;
-    top: 3000px;
+  .scroll-section {
+    /* 1000px margin on top and bottom so only one section can be in view. */
+    margin: 1000px 0px;
   }
-  #more-text {
-    position: absolute;
-    top: 4000px;
+  #hidden {
+    visibility: hidden;
   }
-  #cross-node-context {
-    position: absolute;
-    top: 5000px;
+  #horizontal-scroll {
+    margin-left: 2000px;
   }
-  #text-directive-parameters {
-    position: absolute;
-    top: 6000px;
+  #display-none {
+    display: none;
   }
 </style>
 <body onload="window.requestAnimationFrame(checkScroll)">
-  <div id="element">Element</div>
-  <p id="text">This is a test page !$'()*+./:;=?@_~ &,- &#x2665;</p>
-  <p id="more-text">More test page text</p>
-  <div id="cross-node-context">
+  <div id="element" class="scroll-section">Element</div>
+  <p id="text" class="scroll-section">This is a test page !$'()*+./:;=?@_~ &,- &#x30cd;&#x30b3;</p>
+  <p id="more-text" class="scroll-section">More test page text</p>
+  <div id="cross-node-context" class="scroll-section">
     <div>
       <p>prefix</p>
       <p>test page</p>
     </div>
     <div><p>suffix</p></div>
   </div>
-  <p id="text-directive-parameters">this,is,test,page</p>
+  <p id="text-directive-parameters" class="scroll-section">this,is,test,page</p>
+  <div id="shadow-parent" class="scroll-section"></div>
+  <script>
+    let shadow = document.getElementById("shadow-parent").attachShadow({mode: 'open'});
+    shadow.innerHTML = '<p>shadow text</p>';
+  </script>
+  <p id="hidden" class="scroll-section">hidden text</p>
+  <p id="horizontal-scroll" class="scroll-section">horizontally scrolled text</p>
+  <p id="display-none" class="scroll-section">display none</p>
 </body>
--- a/testing/web-platform/tests/scroll-to-text-fragment/scroll-to-text-fragment.html
+++ b/testing/web-platform/tests/scroll-to-text-fragment/scroll-to-text-fragment.html
@@ -8,189 +8,234 @@
 <script src="/resources/testdriver.js"></script>
 <script src="/resources/testdriver-vendor.js"></script>
 <script>
 let test_cases = [
   // Test non-text fragment directives
   {
     fragment: '#',
     expect_position: 'top',
-    description: 'Empty hash'
+    description: 'Empty hash should scroll to top'
   },
   {
     fragment: '#:~:text=this,is,test,page',
     expect_position: 'top',
-    description: 'Text directive with invalid syntax, context terms without "-"'
+    description: 'Text directive with invalid syntax (context terms without "-") should not parse as a text directive'
   },
   {
     fragment: '#element:~:directive',
     expect_position: 'element',
-    description: 'Generic fragment directive with existing element fragment'
+    description: 'Generic fragment directive with existing element fragment should scroll to element'
+  },
+  {
+    fragment: '#:~:TEXT=test',
+    expect_position: 'top',
+    description: 'Uppercase TEXT directive should not parse as a text directive'
   },
   // Test exact text matching, with all combinations of context terms
   {
     fragment: '#:~:text=test',
     expect_position: 'text',
-    description:  'Exact text with no context'
+    description:  'Exact text with no context should match text'
   },
   {
     fragment: '#:~:text=this is a-,test',
     expect_position: 'text',
-    description: 'Exact text with prefix'
+    description: 'Exact text with prefix should match text'
   },
   {
     fragment: '#:~:text=test,-page',
     expect_position: 'text',
-    description: 'Exact text with suffix'
+    description: 'Exact text with suffix should match text'
   },
   {
     fragment: '#:~:text=this is a-,test,-page',
     expect_position: 'text',
-    description: 'Exact text with prefix and suffix'
+    description: 'Exact text with prefix and suffix should match text'
   },
   // Test text range matching, with all combinations of context terms
   {
     fragment: '#:~:text=this,page',
     expect_position: 'text',
-    description: 'Text range with no context'
+    description: 'Text range with no context should match text'
   },
   {
     fragment: '#:~:text=this-,is,test',
     expect_position: 'text',
-    description: 'Text range with prefix'
+    description: 'Text range with prefix should match text'
   },
   {
     fragment: '#:~:text=this,test,-page',
     expect_position: 'text',
-    description: 'Text range with suffix'
+    description: 'Text range with suffix should match text'
   },
   {
     fragment: '#:~:text=this-,is,test,-page',
     expect_position: 'text',
-    description: 'Text range with prefix and suffix'
+    description: 'Text range with prefix and suffix should match text'
   },
   // Test partially non-matching text ranges
   {
     fragment: '#:~:text=this,none',
     expect_position: 'top',
-    description: 'Text range with non-matching endText'
+    description: 'Text range with non-matching endText should not match'
   },
   {
     fragment: '#:~:text=none,page',
     expect_position: 'top',
-    description: 'Text range with non-matching startText'
+    description: 'Text range with non-matching startText should not match'
   },
   // Test non-matching context terms
   {
     fragment: '#:~:text=this-,is,page,-none',
     expect_position: 'top',
-    description: 'Text range with prefix and nonmatching suffix'
+    description: 'Text range with prefix and nonmatching suffix should not match'
   },
   {
     fragment: '#:~:text=none-,this,test,-page',
     expect_position: 'top',
-    description: 'Text range with nonmatching prefix and matching suffix'
+    description: 'Text range with nonmatching prefix and matching suffix should not match'
   },
   // Test percent encoded characters
   {
     fragment: '#:~:text=this%20is%20a%20test%20page',
     expect_position: 'text',
-    description: 'Exact text with percent encoded spaces'
+    description: 'Exact text with percent encoded spaces should match text'
   },
   {
     fragment: '#:~:text=test%20pag',
     expect_position: 'top',
-    description: 'Non-whole-word exact text with spaces'
+    description: 'Non-whole-word exact text with spaces should not match'
   },
   {
     fragment: '#:~:text=%26%2C%2D',
     expect_position: 'text',
-    description: 'Fragment directive with percent encoded syntactical characters "&,-"'
+    description: 'Fragment directive with percent encoded syntactical characters "&,-" should match text'
   },
   {
-    fragment: '#:~:text=%2665',
+    fragment: '#:~:text=%E3%83%8D%E3%82%B3',
     expect_position: 'text',
-    description: 'Fragment directive with percent encoded non-ASCII unicode character'
+    description: 'Fragment directive with percent encoded non-ASCII unicode character should match text'
   },
   {
     fragment: '#:~:text=!$\'()*+./:;=?@_~',
     expect_position: 'text',
-    description: 'Fragment directive with all TextMatchChars'
+    description: 'Fragment directive with all TextMatchChars should match text'
   },
   // Test multiple text directives
   {
     fragment: '#:~:text=this&text=test,page',
     expect_position: 'text',
-    description: 'Multiple matching exact texts'
+    description: 'Multiple matching exact texts should match text'
   },
   {
     fragment: '#:~:text=tes&text=age',
     expect_position: 'top',
-    description: 'Multiple non-whole-word exact texts'
+    description: 'Multiple non-whole-word exact texts should not match'
+  },
+  {
+    fragment: '#:~:text=none&text=test%20page',
+    expect_position: 'text',
+    description: 'A non-matching text directive followed by a matching text directive should match and scroll into view the second text directive'
+  },
+  {
+    fragment: '#:~:text=test%20page&directive',
+    expect_position: 'text',
+    description: 'Text directive followed by non-text directive should match text'
+  },
+  {
+    fragment: '#:~:text=test&directive&text=page',
+    expect_position: 'text',
+    description: 'Multiple text directives and a non-text directive should match text'
   },
   // Test text directive behavior when there's an element fragment identifier
   {
     fragment: '#element:~:text=test',
     expect_position: 'text',
-    description: 'Text directive with existing element fragment'
+    description: 'Text directive with existing element fragment should match and scroll into view text'
   },
   {
     fragment: '#pagestate:~:text=test',
     expect_position: 'text',
-    description: 'Text directive with nonexistent element fragment'
+    description: 'Text directive with nonexistent element fragment should match and scroll into view text'
   },
   {
     fragment: '#element:~:text=nomatch',
     expect_position: 'element',
-    description: 'Non-matching text directive with existing element fragment'
+    description: 'Non-matching text directive with existing element fragment should scroll to element'
   },
   {
     fragment: '#pagestate:~:text=nomatch',
     expect_position: 'top',
-    description: 'Non-matching text directive with nonexistent element fragment'
+    description: 'Non-matching text directive with nonexistent element fragment should not match and not scroll'
   },
   // Test ambiguous text matches disambiguated by context terms
   {
     fragment: '#:~:text=more-,test%20page',
     expect_position: 'more-text',
-    description: 'Multiple match text directive disambiguated by prefix'
+    description: 'Multiple match text directive disambiguated by prefix should match the prefixed text'
   },
   {
     fragment: '#:~:text=test%20page,-text',
     expect_position: 'more-text',
-    description: 'Multiple match text directive disambiguated by suffix'
+    description: 'Multiple match text directive disambiguated by suffix should match the suffixed text'
   },
   {
     fragment: '#:~:text=more-,test%20page,-text',
     expect_position: 'more-text',
-    description: 'Multiple match text directive disambiguated by prefix and suffix'
+    description: 'Multiple match text directive disambiguated by prefix and suffix should match the text with the given context'
   },
   // Test context terms separated by node boundaries
   {
     fragment: '#:~:text=prefix-,test%20page,-suffix',
     expect_position: 'cross-node-context',
-    description: 'Text directive with context terms separated by node boundaries'
+    description: 'Text directive should match when context terms are separated by node boundaries'
+  },
+  // Test text directive within shadow DOM
+  {
+    fragment: '#:~:text=shadow%20text',
+    expect_position: 'shadow-parent',
+    description: 'Text directive should match text within shadow DOM'
   },
+  // Test text directive within hidden and display none elements. These cases should not scroll into
+  // view, but still "match" in that they should be highlighted or otherwise visibly indicated
+  // if they were to become visible.
+  {
+    fragment: '#:~:text=hidden%20text',
+    expect_position: 'top',
+    description: 'Text directive should not scroll to hidden text'
+  },
+  {
+    fragment: '#:~:text=display%20none',
+    expect_position: 'top',
+    description: 'Text directive should not scroll to display none text'
+  },
+  // Test horizontal scroll into view
+  {
+    fragment: '#:~:text=horizontally%20scrolled%20text',
+    expect_position: 'horizontal-scroll',
+    description: 'Text directive should horizontally scroll into view'
+  }
 ];
 
 for (const test_case of test_cases) {
   promise_test(t => new Promise(resolve => {
     let channel = new BroadcastChannel('scroll-to-text-fragment');
     channel.addEventListener("message", e => {
       resolve(e.data);
     }, {once: true});
 
     test_driver.bless('Open a URL with a text fragment directive', () => {
       window.open('scroll-to-text-fragment-target.html' + test_case.fragment, '_blank', 'noopener');
     });
   }).then(data => {
     assert_equals(data.href.indexOf(':~:'), -1, 'Expected fragment directive to be stripped from the URL.');
     assert_equals(data.scrollPosition, test_case.expect_position,
                   `Expected ${test_case.fragment} (${test_case.description}) to scroll to ${test_case.expect_position}.`);
-  }), `Test navigation with fragment: ${test_case.description}`);
+  }), `Test navigation with fragment: ${test_case.description}.`);
 }
 
 promise_test(t => new Promise(resolve => {
   let channel = new BroadcastChannel('scroll-to-text-fragment');
   channel.addEventListener("message", e => {
     assert_equals(e.data.href.indexOf(':~:'), -1, 'Expected fragment directive to be stripped from the URL.');
 
     // The first navigation has no user activation.