feat(input): a mode that performs hit target check during input #9546
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dgozman
force-pushed
the
input-intercept
branch
from
fac68ea
to
b66efe7
Compare
dgozman
force-pushed
the
input-intercept
branch
2 times, most recently
from
57f6082
to
f725f9b
Compare
dgozman
changed the title
dgozman
force-pushed
the
input-intercept
branch
2 times, most recently
from
6241292
to
d44daa4
Compare
pavelfeldman
reviewed
Nov 5, 2021
dgozman
force-pushed
the
input-intercept
branch
2 times, most recently
from
14dc823
to
3ea19f0
Compare
This replaces previous `checkHitTarget` heuristic that took place before the action
with a new `setupHitTargetInterceptor` that works during the action:
- Before the action we set up capturing listeners on the window.
- During the action we ensure that event target is the element we expect to interact with.
- After the action we clear the listeners.
This should catch the "layout shift" issues where things move
between action point calculation and the actual action.
Possible issues:
- **Risk:** `{ trial: true }` might dispatch move events like `mousemove` or `pointerout`,
because we do actually move the mouse but prevent all other events.
- **Timing**: The timing of "hit target check" has moved, so this may affect different web pages
in different ways, for example expose more races. In this case, we should retry the click as before.
- **No risk**: There is still a possibility of mis-targeting with iframes shifting around,
because we only intercept in the target frame. This behavior does not change.
dgozman
force-pushed
the
input-intercept
branch
from
3ea19f0
to
ffdb471
Compare
dgozman
added a commit
to dgozman/playwright
that referenced
this pull request
Dec 20, 2021
This changes previous layout shift attempt (see microsoft#9546) to account for more valid usecases: - On the first event that is intercepted we enforce the hit target. This is similar to the current mode that checks hit target before the action, but is better timed. - On subsequent events, we only check that target element still covers the action point. If it does not, we run previous logic to determinte the actual hit target. This check is enabled by default, with `process.env.PLAYWRIGHT_NO_LAYOUT_SHIFT_CHECK` to opt out.
dgozman
added a commit
to dgozman/playwright
that referenced
this pull request
Jan 3, 2022
This changes previous layout shift attempt (see microsoft#9546) to account for more valid usecases: - On the first event that is intercepted we enforce the hit target. This is similar to the current mode that checks hit target before the action, but is better timed. - On subsequent events we assume that everything is fine. This covers more scenarios like react rerender, glass pane on mousedown, detach on mouseup. This check is enabled by default, with `process.env.PLAYWRIGHT_NO_LAYOUT_SHIFT_CHECK` to opt out.
dgozman
added a commit
that referenced
this pull request
Jan 4, 2022
This changes previous layout shift attempt (see #9546) to account for more valid usecases: - On the first event that is intercepted we enforce the hit target. This is similar to the current mode that checks hit target before the action, but is better timed. - On subsequent events we assume that everything is fine. This covers more scenarios like react rerender, glass pane on mousedown, detach on mouseup. This check is enabled by default, with `process.env.PLAYWRIGHT_NO_LAYOUT_SHIFT_CHECK` to opt out.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reincarnation of #6352.
This change replaces previous
checkHitTargetheuristic that took place before the action with a newsetupHitTargetInterceptorthat works during the action:This should catch the "layout shift" issues where things move between action point calculation and the actual action.
Possible issues:
{ trial: true }might dispatch move events likemousemoveorpointerout,because we do actually move the mouse but prevent all other events.
in different ways, for example expose more races. In this case, we should retry the click as before.
because we only intercept in the target frame. This behavior does not change.
There is an opt-out environment variable
PLAYWRIGHT_NO_LAYOUT_SHIFT_CHECKthat reverts to previous behavior.Fixes #6238.