Bug 1594269 [wpt PR 20105] - Improve scroll to text WPT coverage., a=testonly
authorNick Burris <nburris@chromium.org>
Mon, 25 Nov 2019 16:46:29 +0000
changeset 504440 2586997ecf543fb7ce068caf231199897fb2b9c0
parent 504439 752bae5314631afb8a14a0642404d9fa2548a60f
child 504441 bf228d83752f337e3a2a958410dbabb894345044
push id36862
push useraciure@mozilla.com
push dateFri, 29 Nov 2019 21:26:53 +0000
treeherdermozilla-central@b4b10ae558b9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstestonly
bugs1594269, 20105, 1900648, 713057
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 1594269 [wpt PR 20105] - Improve scroll to text WPT coverage., a=testonly Automatic update from web-platform-tests Improve scroll to text WPT coverage. Adds many tests to the scroll to text WPT suite: - Setting window.location.fragmentDirective has no effect - All combinations of optional parameters in text directive - Matching TextMatchChars and PercentEncodedChars (in particular the syntactical characters '&', ‘,’ and ‘-’) including non-ASCII - Multiple matches in the page - Cross-whitespace/node matching (i.e. context terms and match terms can be separated by whitespace and node boundaries) Also added a readable description to each test case. Note all tests pass in Chrome except for non-ASCII character matching. Change-Id: I2a692049ba81bef5412e7b73909fbcdc710eb0da Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1900648 Commit-Queue: Nick Burris <nburris@chromium.org> Reviewed-by: David Bokan <bokan@chromium.org> Cr-Commit-Position: refs/heads/master@{#713057} -- wpt-commits: 186457c2f6c0f52d9939cddeefeaebcff38d681b wpt-pr: 20105
testing/web-platform/tests/scroll-to-text-fragment/scroll-to-text-fragment-api.html
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
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/scroll-to-text-fragment/scroll-to-text-fragment-api.html
@@ -0,0 +1,32 @@
+<!doctype html>
+<title>Fragment directive API</title>
+<meta charset=utf-8>
+<link rel="help" href="https://wicg.github.io/ScrollToTextFragment/">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/testdriver.js"></script>
+<script src="/resources/testdriver-vendor.js"></script>
+<script>
+test(t => {
+  assert_equals(typeof(window.location.fragmentDirective), 'object', 'window.location.fragmentDirective is defined');
+}, 'Scroll to text is feature detectable via window.location.fragmentDirective');
+
+test(t =>{
+  window.location.fragmentDirective = 'text=test';
+  assert_equals(window.scrollY, 0, 'Setting window.location.fragmentDirective did not have an effect on scroll position');
+  assert_equals(typeof(window.location.fragmentDirective), 'object', 'window.location.fragmentDirective is still an object type');
+  assert_equals(Object.keys(window.location.fragmentDirective).length, 0, 'window.location.fragmentDirective has no properties');
+}, 'Setting window.location.fragmentDirective has no effect');
+</script>
+<style>
+  body {
+    height: 3200px;
+  }
+  #text {
+    position: absolute;
+    top: 3000px;
+  }
+</style>
+<body>
+  <p id="text">This is a test page</p>
+</body>
--- 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
@@ -11,31 +11,58 @@ function checkScroll() {
 
   let position = 'unknown';
   if (window.scrollY == 0)
     position = 'top';
   else if (isInView(document.getElementById('element')))
     position = 'element';
   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';
 
   bc.postMessage({ scrollPosition: position, href: window.location.href });
   bc.close();
   window.close();
 }
 </script>
 <style>
   body {
-    height: 3200px;
-  }
-  p {
-    position: absolute;
-    top: 3000px;
+    height: 6200px;
   }
   #element {
     position: absolute;
     top: 2000px;
   }
+  #text {
+    position: absolute;
+    top: 3000px;
+  }
+  #more-text {
+    position: absolute;
+    top: 4000px;
+  }
+  #cross-node-context {
+    position: absolute;
+    top: 5000px;
+  }
+  #text-directive-parameters {
+    position: absolute;
+    top: 6000px;
+  }
 </style>
 <body onload="window.requestAnimationFrame(checkScroll)">
   <div id="element">Element</div>
-  <p id="text">This is a test page</p>
+  <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>
+      <p>prefix</p>
+      <p>test page</p>
+    </div>
+    <div><p>suffix</p></div>
+  </div>
+  <p id="text-directive-parameters">this,is,test,page</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
@@ -4,51 +4,193 @@
 <link rel="help" href="https://wicg.github.io/ScrollToTextFragment/">
 <meta name="timeout" content="long">
 <script src="/resources/testharness.js"></script>
 <script src="/resources/testharnessreport.js"></script>
 <script src="/resources/testdriver.js"></script>
 <script src="/resources/testdriver-vendor.js"></script>
 <script>
 let test_cases = [
-  { fragment: '#', expect_position: 'top' },
-  { fragment: '#:~:text=test', expect_position: 'text' },
-  { fragment: '#:~:text=this,page', expect_position: 'text' },
-  { fragment: '#:~:text=this-,is,test', expect_position: 'text' },
-  { fragment: '#:~:text=this-,is,test,-page', expect_position: 'text' },
-  { fragment: '#:~:text=this-,is,page,-none', expect_position: 'top' },
-  { fragment: '#:~:text=this,test,-page', expect_position: 'text' },
-  { fragment: '#:~:text=this%20is%20a%20test%20page', expect_position: 'text' },
-  { fragment: '#:~:text=this&text=test,page', expect_position: 'text' },
-  { fragment: '#:~:text=tes&text=age', expect_position: 'top' },
-  { fragment: '#pagestate:~:text=test', expect_position: 'text' },
-  { fragment: '#pagestate:~:text=nomatch', expect_position: 'top' },
-  { fragment: '#element:~:text=nomatch', expect_position: 'element' },
-  { fragment: '#element:~:directive', expect_position: 'element' },
+  // Test non-text fragment directives
+  {
+    fragment: '#',
+    expect_position: 'top',
+    description: 'Empty hash'
+  },
+  {
+    fragment: '#:~:text=this,is,test,page',
+    expect_position: 'top',
+    description: 'Text directive with invalid syntax, context terms without "-"'
+  },
+  {
+    fragment: '#element:~:directive',
+    expect_position: 'element',
+    description: 'Generic fragment directive with existing element fragment'
+  },
+  // Test exact text matching, with all combinations of context terms
+  {
+    fragment: '#:~:text=test',
+    expect_position: 'text',
+    description:  'Exact text with no context'
+  },
+  {
+    fragment: '#:~:text=this is a-,test',
+    expect_position: 'text',
+    description: 'Exact text with prefix'
+  },
+  {
+    fragment: '#:~:text=test,-page',
+    expect_position: 'text',
+    description: 'Exact text with suffix'
+  },
+  {
+    fragment: '#:~:text=this is a-,test,-page',
+    expect_position: 'text',
+    description: 'Exact text with prefix and suffix'
+  },
+  // Test text range matching, with all combinations of context terms
+  {
+    fragment: '#:~:text=this,page',
+    expect_position: 'text',
+    description: 'Text range with no context'
+  },
+  {
+    fragment: '#:~:text=this-,is,test',
+    expect_position: 'text',
+    description: 'Text range with prefix'
+  },
+  {
+    fragment: '#:~:text=this,test,-page',
+    expect_position: 'text',
+    description: 'Text range with suffix'
+  },
+  {
+    fragment: '#:~:text=this-,is,test,-page',
+    expect_position: 'text',
+    description: 'Text range with prefix and suffix'
+  },
+  // Test partially non-matching text ranges
+  {
+    fragment: '#:~:text=this,none',
+    expect_position: 'top',
+    description: 'Text range with non-matching endText'
+  },
+  {
+    fragment: '#:~:text=none,page',
+    expect_position: 'top',
+    description: 'Text range with non-matching startText'
+  },
+  // Test non-matching context terms
+  {
+    fragment: '#:~:text=this-,is,page,-none',
+    expect_position: 'top',
+    description: 'Text range with prefix and nonmatching suffix'
+  },
+  {
+    fragment: '#:~:text=none-,this,test,-page',
+    expect_position: 'top',
+    description: 'Text range with nonmatching prefix and matching suffix'
+  },
+  // Test percent encoded characters
+  {
+    fragment: '#:~:text=this%20is%20a%20test%20page',
+    expect_position: 'text',
+    description: 'Exact text with percent encoded spaces'
+  },
+  {
+    fragment: '#:~:text=test%20pag',
+    expect_position: 'top',
+    description: 'Non-whole-word exact text with spaces'
+  },
+  {
+    fragment: '#:~:text=%26%2C%2D',
+    expect_position: 'text',
+    description: 'Fragment directive with percent encoded syntactical characters "&,-"'
+  },
+  {
+    fragment: '#:~:text=%2665',
+    expect_position: 'text',
+    description: 'Fragment directive with percent encoded non-ASCII unicode character'
+  },
+  {
+    fragment: '#:~:text=!$\'()*+./:;=?@_~',
+    expect_position: 'text',
+    description: 'Fragment directive with all TextMatchChars'
+  },
+  // Test multiple text directives
+  {
+    fragment: '#:~:text=this&text=test,page',
+    expect_position: 'text',
+    description: 'Multiple matching exact texts'
+  },
+  {
+    fragment: '#:~:text=tes&text=age',
+    expect_position: 'top',
+    description: 'Multiple non-whole-word exact texts'
+  },
+  // 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'
+  },
+  {
+    fragment: '#pagestate:~:text=test',
+    expect_position: 'text',
+    description: 'Text directive with nonexistent element fragment'
+  },
+  {
+    fragment: '#element:~:text=nomatch',
+    expect_position: 'element',
+    description: 'Non-matching text directive with existing element fragment'
+  },
+  {
+    fragment: '#pagestate:~:text=nomatch',
+    expect_position: 'top',
+    description: 'Non-matching text directive with nonexistent element fragment'
+  },
+  // 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'
+  },
+  {
+    fragment: '#:~:text=test%20page,-text',
+    expect_position: 'more-text',
+    description: 'Multiple match text directive disambiguated by suffix'
+  },
+  {
+    fragment: '#:~:text=more-,test%20page,-text',
+    expect_position: 'more-text',
+    description: 'Multiple match text directive disambiguated by prefix and suffix'
+  },
+  // 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'
+  },
 ];
 
-test(t => {
-  assert_equals(typeof(window.location.fragmentDirective), 'object', 'window.location.fragmentDirective is defined');
-}, 'Scroll to text is feature detectable via window.location.fragmentDirective');
-
 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 + ' to scroll to ' + test_case.expect_position);
-  }), 'Test navigation with text fragment directive ' + test_case.fragment);
+                  `Expected ${test_case.fragment} (${test_case.description}) to scroll to ${test_case.expect_position}.`);
+  }), `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.