Clipboard API: use bless() instead #37098
Conversation
|
@marcoscaceres looking at the test results here, it looks like 3 tests have gone missing: The same can be seen for Chrome and Firefox. Can you check what might be going on with this? |
marcoscaceres
force-pushed
the
clipboard_bless
branch
from
1f238b3 to
e1a04ee
Compare
|
Let's see if the strange missing tests situation corrects itself... not sure why that would happen. |
clipboard-apis/async-custom-formats-write-read.tentative.https.html
Outdated
Show resolved
Hide resolved
clipboard-apis/async-navigator-clipboard-read-resource-load.https.html
Outdated
Show resolved
Hide resolved
clipboard-apis/async-unsanitized-html-formats-write-read.tentative.https.html
Outdated
Show resolved
Hide resolved
clipboard-apis/async-unsanitized-plaintext-formats-write-read.tentative.https.html
Outdated
Show resolved
Hide resolved
clipboard-apis/detached-iframe/write-read-on-detached-iframe.https.html
Outdated
Show resolved
Hide resolved
clipboard-apis/detached-iframe/writeText-readText-on-detached-iframe.https.html
Outdated
Show resolved
Hide resolved
...ead/clipboard-read-enabled-by-feature-policy-attribute-cross-origin-tentative.https.sub.html
Outdated
Show resolved
Hide resolved
...y/clipboard-read/clipboard-read-enabled-by-feature-policy-attribute-tentative.https.sub.html
Outdated
Show resolved
Hide resolved
...lipboard-read/clipboard-read-enabled-by-feature-policy-cross-origin-tentative.https.sub.html
Outdated
Show resolved
Hide resolved
...ture-policy/clipboard-read/clipboard-read-enabled-by-feature-policy.tentative.https.sub.html
Outdated
Show resolved
Hide resolved
...pboard-read/clipboard-read-enabled-on-self-origin-by-feature-policy.tentative.https.sub.html
Outdated
Show resolved
Hide resolved
...te/clipboard-write-enabled-by-feature-policy-attribute-cross-origin-tentative.https.sub.html
Outdated
Show resolved
Hide resolved
...clipboard-write/clipboard-write-enabled-by-feature-policy-attribute-tentative.https.sub.html
Outdated
Show resolved
Hide resolved
clipboard-apis/text-write-read/async-writeText-readText.https.html
Outdated
Show resolved
Hide resolved
...pboard-write/clipboard-write-enabled-by-feature-policy-cross-origin-tentative.https.sub.html
Outdated
Show resolved
Hide resolved
clipboard-apis/async-navigator-clipboard-read-sanitize.https.html
Outdated
Show resolved
Hide resolved
|
@saschanaz, hopefully better now 🙈 |
|
Hehe, I like letting the bots do the work🤖 will check, don’t worry. |
|
(You might want to keep the load event listener inside the current waitForUserActivation?) |
|
Why not just replace the implementation of waitForUserActivation is far more readable in the test - how many people know that test_driver.bless() means the test waits for user activation? I know I wasn't aware of that. See also saschanaz's comment, since keeping waitForUserActivation would make it easier to keep the 'load' listener, and also add workarounds if in the future we find a scenario where |
That adds additional layers of indirection: test ➡️ user-activation.js ➡️ test_driver...
Right, but it's the same problem with
I understand. That should have been caught in review (and it's part of all of us learning what the test_driver API provides). This was caught because it then goes into WebKit's review.
Right, but we should only use something like waitForUserActivation() in cases where bless() doesn't do what we need. The load listener might be a more general issue with the test design, which we hopefully shouldn't need at all. I'll take a look. |
marcoscaceres
force-pushed
the
clipboard_bless
branch
from
ef38fd5 to
739a448
Compare
|
Ok, so, noting that bless() supports user activation within iframes. The load was using the wait for load on the iframes, but it's less racy to just set that explicitly. I've updated the tests to do that... |
| } | ||
| }); | ||
| </script> | ||
| </html> |
There was a problem hiding this comment.
Don't need this... there's no matching head one either.
|
@saschanaz, @garykac, let me know what you think! Hopefully this captures the intent better. |
marcoscaceres
force-pushed
the
clipboard_bless
branch
from
e14e1b7 to
a12c1d4
Compare
| await new Promise(resolve => { | ||
| iframe.onload = resolve; | ||
| iframe.src = 'about:blank'; | ||
| }); |
There was a problem hiding this comment.
Hmm? Are we sure loading about:blank is synchronous everywhere? The existing wait code in waitForUserActivation is better IMO.
And we probably should still have a helper function for this.
| const iframeClipboard = iframe.contentWindow.navigator.clipboard; | ||
| const blobInput = new Blob(['test string'], {type: 'text/plain'}); | ||
| const clipboardItemInput = new ClipboardItem({'text/plain': blobInput}); | ||
| await waitForUserActivation(); | ||
| await test_driver.bless("clipboard", null, iframe.contentWindow); |
There was a problem hiding this comment.
Wouldn't this change the test behavior here? Previously it clicked the parent page and now this clicks the iframe. wpt.fyi reports no change though.
And should the intent argument be passed consistently?
|
Ok, closing this as I'm not getting time to go through them properly. |
Replaces
waitForUserActivation()fortest_driver.bless(). They functionally do the same thing, but bless is better for devices where an actual clickable thing needs to be activated.