Fix element hash fallback in iframes

The text directive element-id fallback tries to scroll to the element-id if a text match wasn't found. It does so in PerformPreRafActions since element fragments cause focus which can execute script. Invoke(), where the text fragment performs its text search, occurs from lifecycle updates where script execution is forbidden. Text directives used to be limited to main-frames but were enabled explicitly for iframes in https://crrev.com/a71830a90b72f0a613fe9. However, PerformPreRafActions wasn't updated to call into the FragmentAnchor of an iframe We also need to strip the fragment directives in ElementFragmentAnchor::TryCreate. In cases where a text fragment is successfully created, the element id fallback works by creating an ElementFragmentAnchor from inside TextFragmentAnchor. However, if the TextFragmentAnchor is blocked or not parsed correctly, the URL passed to ElementFragmentAnchor in FragmentDirective::TryCreate will still include the directives. A test for this was added in the new same-document.tests.html Bug: 1260135 Change-Id: Ib3f19b462016d34cc41d21b05933aa9cb822a9b8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3224519 Commit-Queue: David Bokan <[email protected]> Reviewed-by: Robert Flack <[email protected]> Cr-Commit-Position: refs/heads/main@{#933575}
web-platform-tests · Oct 20, 2021 · c8131af · c8131af
1 parent 5f0a02b
commit c8131af
Show file tree

Hide file tree

Showing 3 changed files with 122 additions and 0 deletions.
diff --git a/scroll-to-text-fragment/iframe-target.html b/scroll-to-text-fragment/iframe-target.html
@@ -13,6 +13,8 @@
       position = 'top';
     else if (isInView(document.getElementById('target')))
       position = 'target';
+    else if (isInView(document.getElementById('elementid')))
+      position = 'elementid';
 
     let results = {
       scrollPosition: position,
@@ -29,6 +31,10 @@
       window.requestAnimationFrame(() => {
         window.requestAnimationFrame(postResult);
       })
+    } else if (e.data == 'reset') {
+      window.location.hash = '';
+      window.scrollTo(0, 0);
+      window.top.postMessage('', "*");
     }
   });
 </script>
@@ -40,4 +46,5 @@
 </style>
 <body>
   <p id="target">Target Text</p>
+  <div id="elementid">DIV</div>
 </body>
diff --git a/scroll-to-text-fragment/iframes.sub.html b/scroll-to-text-fragment/iframes.sub.html
@@ -14,6 +14,15 @@
     });
   }
 
+  function reset(iframe) {
+    return new Promise((resolve) => {
+      window.addEventListener('message', (e) => {
+        resolve();
+      }, {once: true});
+      iframe.contentWindow.postMessage('reset', '*');
+    });
+  }
+
   function runTests() {
     const attribute_iframe = document.getElementById('srcattrib');
     const sameorigin_iframe = document.getElementById('sameorigin');
@@ -22,6 +31,7 @@
     // Check behavior that occurs when a text fragment is specified directly in
     // an iframe's src attribute. We expect the text fragment to be blocked.
     promise_test(t => new Promise(async resolve => {
+        // No reset since we're checking the hash specified in the `src` attribute.
         const data = await getResult(attribute_iframe);
         resolve(data);
       }).then( data => {
@@ -34,6 +44,7 @@
     // a same-document navigation initiated by an origin that's same-origin
     // with the current document.
     promise_test(t => new Promise(async resolve => {
+        await reset(sameorigin_iframe);
         sameorigin_iframe.contentWindow.location = `${sameorigin_iframe.src}#:~:text=Target`;
         const data = await getResult(sameorigin_iframe);
         resolve(data);
@@ -46,13 +57,37 @@
     // cross-origin parent. The text fragment should be blocked because the
     // initiating origin is not same-origin with the current document.
     promise_test(t => new Promise(async resolve => {
+        await reset(crossorigin_iframe);
         crossorigin_iframe.contentWindow.location = `${crossorigin_iframe.src}#:~:text=Target`;
         const data = await getResult(crossorigin_iframe);
         resolve(data);
       }).then( data => {
         assert_equals(data.href.indexOf(':~:'), -1, 'Expected fragment directive to be stripped from the URL.');
         assert_equals(data.scrollPosition, 'top', 'Should not to scroll to text fragment.');
     }), 'Navigate cross-origin iframe via window.location');
+
+    // Check the element-id fallback behavior when the text is not found. We
+    // should fallback to a regular element-id hash navigation.
+    promise_test(t => new Promise(async resolve => {
+        await reset(sameorigin_iframe);
+        sameorigin_iframe.contentWindow.location = `${sameorigin_iframe.src}#elementid:~:text=NonExistentText`;
+        const data = await getResult(sameorigin_iframe);
+        resolve(data);
+      }).then( data => {
+        assert_equals(data.scrollPosition, 'elementid', 'Should scroll to the element-id anchor.');
+    }), 'Non-matching text with element-id fallback');
+
+    // Check the element-id fallback behaviour when used across origins. The
+    // text fragment should be blocked across origins but the element id hash
+    // should not.
+    promise_test(t => new Promise(async resolve => {
+        await reset(crossorigin_iframe);
+        crossorigin_iframe.contentWindow.location = `${crossorigin_iframe.src}#elementid:~:text=Target%20Text`;
+        const data = await getResult(crossorigin_iframe);
+        resolve(data);
+      }).then( data => {
+        assert_equals(data.scrollPosition, 'elementid', 'Should scroll to the element-id anchor.');
+    }), 'Cross-origin with element-id fallback');
   }
 </script>
 <style>

diff --git a/scroll-to-text-fragment/same-document-tests.html b/scroll-to-text-fragment/same-document-tests.html
@@ -0,0 +1,80 @@
+<!doctype html>
+<title>Same document navigation to text fragment directives</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>
+  function reset() {
+    location.hash = '';
+    window.scrollTo(0, 0);
+  }
+
+  function isInViewport(element) {
+    const viewportRect = {
+      left: visualViewport.offsetLeft,
+      top: visualViewport.offsetTop,
+      right: visualViewport.offsetLeft + visualViewport.width,
+      bottom: visualViewport.offsetTop + visualViewport.height
+    };
+
+    const elementRect = element.getBoundingClientRect();
+    const elementCenter = {
+      x: elementRect.left + elementRect.width / 2,
+      y: elementRect.top + elementRect.height / 2
+    };
+
+    return elementCenter.x > viewportRect.left &&
+           elementCenter.x < viewportRect.right &&
+           elementCenter.y > viewportRect.top &&
+           elementCenter.y < viewportRect.bottom;
+  }
+
+  function runTests() {
+    // Ensure a simple text directive works correctly when navigated to the
+    // same document using `location.hash`.
+    promise_test(async t => {
+      assert_implements(document.fragmentDirective, 'Text directive not implemented');
+      reset();
+
+      location.hash = ':~:text=line%20of%20text';
+      await t.step_wait(() => window.scrollY > 0, "Wait for scroll");
+      assert_true(isInViewport(document.getElementById('text')), 'Scrolled to text');
+    }, 'Basic text directive navigation');
+
+    // Test that we correctly fallback to the element id when we have a text
+    // directive that doesn't match any text in the page.
+    promise_test(async t => {
+      assert_implements(document.fragmentDirective, 'Text directive not implemented');
+      reset();
+
+      location.hash = 'elementid:~:text=textDoesntExist';
+      await t.step_wait(() => window.scrollY > 0, "Wait for scroll");
+      assert_true(isInViewport(document.getElementById('elementid')), 'Scrolled to `elementid`');
+    }, 'Basic element id fallback');
+
+    // Test that we correctly fallback to the element id when we have a text
+    // directive that's malformed and won't be parsed.
+    promise_test(async t => {
+      assert_implements(document.fragmentDirective, 'Text directive not implemented');
+      reset();
+
+      location.hash = 'elementid:~:text=,,,,,';
+      await t.step_wait(() => window.scrollY > 0, "Wait for scroll");
+      assert_true(isInViewport(document.getElementById('elementid')), 'Scrolled to `elementid`');
+    }, 'Malformed text directive element id fallback');
+  }
+</script>
+<style>
+  div {
+    margin: 200vh 0 200vh 0;
+  }
+</style>
+<body onload="runTests()">
+  <div id="text">
+    This is a line of text.
+  </div>
+  <div id="elementid">
+    This div has an id: 'elementid'.
+  </div>
+</body>