Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Cannot test ::target-text rendering #26364

Open
lilles opened this issue Nov 2, 2020 · 8 comments
Open

Cannot test ::target-text rendering #26364

lilles opened this issue Nov 2, 2020 · 8 comments
Labels
infra priority:backlog type:untestable

Comments

@lilles
Copy link
Member

lilles commented Nov 2, 2020

There are a lot of limitations on when we are allowed to use text fragment for navigation:

https://wicg.github.io/scroll-to-text-fragment/#security-and-privacy

I have not found a way to make a ref-test within these limitations with the current wpt framework. Testing locally, I am able to do so by adding support for meta variants for reftests. That is, add and add the first variant to the rel_url here:
https://source.chromium.org/chromium/chromium/src/+/master:third_party/blink/tools/blinkpy/third_party/wpt/wpt/tools/manifest/sourcefile.py;l=1107

For some reason, the -ref.html would not generate a screen-shot with that change, but it's probably a simple fix for it.

Could we consider supporting variants, or a simple fragment addition to reftests?
@lilles
Copy link
Member Author

lilles commented Nov 3, 2020

I made this patch to allow me to work on test-cases, but it's by no means PR-ready:

https://chromium-review.googlesource.com/c/chromium/src/+/2517060

It introduces support for a meta tag for fragments:

<meta name="fragment" content="#frag">
<meta name="fragment" content="#:~:text=match">

@Hexcles
Copy link
Member

Hexcles commented Nov 11, 2020

Sorry for the late reply. To capture the discussion on Slack, there are three approaches considered so far:

  1. Set window.location or use window.open, both of which require testdriver.bless to make scrolling to text work due to the security limitations. See Support testdriver.js in reftests #13183 for supporting testdriver in reftests.
  2. Use http-equiv=refresh which seems to break the test runner.
  3. Add a new meta tag for fragments similar to variants.

My hunch is that option 3 is the easiest.

@lilles
Copy link
Member Author

lilles commented Nov 13, 2020 •
edited
Loading

It looks like this should be possible with <meta name="variant"> if we add support for adding variant in ref-tests, instead of adding the new fragment meta. I ran into some issues, but I don't remember what that was now.

@lilles
Copy link
Member Author

lilles commented Nov 13, 2020

This currently works for testharness tests, at least in Chrome:

<!doctype html>
<meta name="variant" content="#:~:text=match">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<div style="height:5000px"></div>
<div id="target">match</div>
<script>
  async_test(t => {
    document.addEventListener("scroll", t.step_func_done(() => {}));
  });
</script>

Adding a loop over RefTests adding the variants at the end of the url like we do for TestHarness makes us run the reftests correctly in Chrome, but with missing expectation file. There's probably a simple fix for that.

@lilles
Copy link
Member Author

lilles commented Nov 13, 2020

I just needed to strip the query off the path for references to make it work with variants: https://chromium-review.googlesource.com/c/chromium/src/+/2537679

@lilles
Copy link
Member Author

lilles commented Nov 16, 2020

Added an RFC PR here: web-platform-tests/rfcs#71

@stephenmcgruer
Copy link
Contributor

Keeping this issue up-to-date: The RFC got a bit bogged down, but in late December we agreed on a path forward. We are now waiting for @lilles to update it to encompass the option that WPT core team could agree on (web-platform-tests/rfcs#71 (comment)).

@mrego
Copy link
Member

mrego commented Oct 1, 2021

It seems it's possible to write tests for ::target-text with location.href (see https://github.com/web-platform-tests/wpt/blob/master/css/css-highlight-api/painting/custom-highlight-painting-below-target-text.html).

@gsnedders gsnedders mentioned this issue Aug 2, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
infra priority:backlog type:untestable
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@mrego @past @stephenmcgruer @lilles @Hexcles @LukeZielinski