search

oddbird / css-anchor-positioning

Polyfill for CSS Anchor Positioning
https://anchor-polyfill.netlify.app/
BSD 3-Clause "New" or "Revised" License
250 stars 8 forks source link

Pseudo-element anchors #213

Closed ayoreis closed 2 months ago

ayoreis commented 3 months ago

TODO:

~Performance does not seem worse (tested with console.time consistently at \~110ms).~

Closes #208.

netlify[bot] commented 3 months ago

Deploy Preview for anchor-position-wpt canceled.

Name Link
Latest commit 1483a7672bc1565f9ea202582a49673586395c82
Latest deploy log https://app.netlify.com/sites/anchor-position-wpt/deploys/669c1677c064780008ba56de
netlify[bot] commented 3 months ago

Deploy Preview for anchor-polyfill ready!

Name Link
Latest commit 1483a7672bc1565f9ea202582a49673586395c82
Latest deploy log https://app.netlify.com/sites/anchor-polyfill/deploys/669c16770044a900089e85d6
Deploy Preview https://deploy-preview-213--anchor-polyfill.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

ayoreis commented 2 months ago

I updated the tests and fixed @position-fallback. Also now instead of inserting the pseudo-element's content as text into the fake pseudo-element, it add it to the ::before of the fake pseudo-element supporting all types (eg. images).

Currently all these changes add about 15ms to the polyfill (on the homepage, on my machine). It seem that the increase is only because of enabling parseRulePrelude for csstree, so it won't increase much with the amount of pseudo-element anchors, so that amount seems fine.

I looked into WPT, the problem is that --inject-script is sync and we have async code, so WPT runs before everything has been polyfilled, I think window.CHECK_LAYOUT_DELAY in src/index-wpt.ts should fix this but I can't find any docs on it to continue debugging.

One unrelated WPT test started passing with these changes (/css/css-anchor-position/anchor-query-custom-property-registration.html).

jamesnw commented 2 months ago

I looked into WPT, the problem is that --inject-script is sync and we have async code, so WPT runs before everything has been polyfilled, I think window.CHECK_LAYOUT_DELAY in src/index-wpt.ts should fix this but I can't find any docs on it to continue debugging.

window.CHECK_LAYOUT_DELAY was added to the WPT in order to make this polyfill testable. It is currently used in checkLayoutForAnchorPos to add a delay of 3 animation frames before checking layout. See this comment for more info.

The pseudoelement tests are not using checkLayoutForAnchorPos, and so I think we'll need another strategy. Perhaps we add a function to WPT that we can use to delay other tests that aren't using checkLayoutForAnchorPos? I'm guessing this is also the case for other test files as well.

ayoreis commented 2 months ago

I'm not familiar with the WPT codebase, but could <script type="module"> injected scripts be implemented? If yes, I might be interested in working on it with some guidance.

jamesnw commented 2 months ago

I'm not familiar with the WPT codebase, but could <script type="module"> injected scripts be implemented? If yes, I might be interested in working on it with some guidance.

Yeah- there are some <script type="module"> examples on other anchor position tests. I think you can run ./wpt serve --inject-script=[polyfillpath] and run it locally (note serve rather than run in this context. You can navigate to http://web-platform.test:8000/css/css-anchor-position/pseudo-element-anchor.html for example and that should have the polyfill loaded.

ayoreis commented 2 months ago

I think I didn't explain well. What I meant is: Is it possible to implement support for injecting type="module" scripts that run before the test, for example this is the resulting test with --inject-script:

<!DOCTYPE html>
<script>
await console.log('Hello') // throws

// Remove the injected script tag from the DOM.
document.currentScript.remove();
</script>

<title>Test</title>

<div id="element">Test</div>

<script>
  test(() => {
    assert_equals(element.textContent, 'Test');
  }, 'Test');
</script>

And with something like --inject-script-module, if it could be implemented:

<!DOCTYPE html>
<title>Test</title>

<div id="element">Test</div>

<script type="module">
await console.log('Hello') 

// Remove the injected script tag from the DOM.
document.currentScript.remove();
</script>

<script>
  test(() => {
    assert_equals(element.textContent, 'Test');
  }, 'Test');
</script>

This would solve our use case.

A issue with this idea is that all tests would have to be converted to <script type="module">s (because type="module" scripts defer), a workaround could be for the test function to wrap the callback in an addEventListener('load'), but I don't know if that would cover all cases.

jamesnw commented 2 months ago

I think I didn't explain well.

No- you explained well, on my second reading I realized I had misunderstood, but you got your response up first.

To boil down the problem, the tests are running before the polyfill is complete. Ideally, we would add some delays for testing the polyfill that wouldn't impact non-polyfill tests. I don't think that adding an injected module script would be simple.

We do have a few tools at our disposal- pseudo-element-anchor.html works if you use promise_test and waitForAtLeastOneFrame()-

<script>
  promise_test(async () => {
    await waitForAtLeastOneFrame();
    assert_equals(anchored1.offsetLeft, 100);
    assert_equals(anchored1.offsetTop, 100);
  }, "::before as anchor");
  promise_test(async () => {
    await waitForAtLeastOneFrame();
    assert_equals(anchored2.offsetLeft, 100);
    assert_equals(anchored2.offsetTop, 100);
  }, "::after as anchor");
</script>

But that also impacts non-polyfilled tests. @jgerigmeyer Do you have any thoughts on an analog to the window.checkLayoutForAnchorPos might be? I'm wondering about a waitForAtLeastOneFrame wrapper that only waits if CHECK_LAYOUT_DELAY is true?

jgerigmeyer commented 2 months ago

@jgerigmeyer Do you have any thoughts on an analog to the window.checkLayoutForAnchorPos might be? I'm wondering about a waitForAtLeastOneFrame wrapper that only waits if CHECK_LAYOUT_DELAY is true?

@jamesnw Yeah, when we landed https://github.com/web-platform-tests/wpt/pull/38442 we knew that there were a few WPTs that called test() directly, and it didn't fix those cases. We might want to check with mfreed7 to see what the WPT folks would be open to, but I could see adding something similar to checkLayoutForAnchorPos that would replace test() for anchor-position tests only.

jamesnw commented 2 months ago

I opened a PR on oddbird:wpt https://github.com/oddbird/wpt/pull/3/files that adds a test function which waits for the polyfill to run. It makes pseudo-element-anchor.html pass, but doesn't work on pseudo-element-anchor-dynamic.html because dynamic changes aren't yet supported.

If this seems like it will fix this, I'll check with mfreed7 and work on upstreaming.

jgerigmeyer commented 2 months ago

@ayoreis I'm going to go ahead and merge this -- thanks!

I think the only outstanding somewhat-related item I see is this, but it's not urgent/blocking:

At some point, could you also open issues for the things we are not covering here- other pseudo elements like the file upload input button, or scrolling?