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.

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

Add range input recording support #28767

Merged
merged 1 commit into from
Dec 31, 2023

Conversation

ruifigueira
Copy link
Contributor

@ruifigueira ruifigueira commented Dec 22, 2023

Playwright doesn't record range inputs because it consumes browser events to prevent the native action execution, and instead triggers a playwright action to replicate that same event. However, things like slider drag and drop on a range input are not properly replicated. That causes the slider not to move at all, because playwright just sends a click action into the center of the slider, while preventing me from manually sliding to the desired value:

chrome_ahWgoKXidX

To try it, just run:

npx playwright codegen 'data:text/html,<input type="range" min="0" max="10" value="5">' 

My approach was to discard the playwright action, and instead let all events do their thing, without consuming them. This simplifies the recording logic and the recorded page behaves much better, because it is handling all eventss in the exact same way as if the user was not recording at all. And with this approach, recording a range input automatically works. I just added a few more logic to skip the click action recording there, but that's basically it.

Here's a video of it working with the new changes:

chrome_i8oDvD74Un

I ran all tests in tests/library/inspector, and they all pass with just a minor change in one of the tests (playwright action was triggering more events than the actual browser events).

This comment has been minimized.

@pavelfeldman
Copy link
Member

You probably want to replace one line and add the range to the list here:

if (nodeName === 'INPUT' && ['date'].includes((target as HTMLInputElement).type))

We like it when actions are performed by Playwright because it gives us more confidence that the recorded script will play.

@ruifigueira
Copy link
Contributor Author

ruifigueira commented Dec 22, 2023

You probably want to replace one line and add the range to the list here:

True 😁 I already reverted all the other bits and left that change only.

We like it when actions are performed by Playwright because it gives us more confidence that the recorded script will play.

I thought it so, and I understand. However, I already hit some scenarios where the page behaved poorly when recording was enabled. For instance, when clicking some button on a scrollable area, playwright would start scrolling and click which was very strange (I know it now, it is playwright doing that magic, but nevertheless). And overall, interactions don't seem fluid in more complex pages, at least it's my feeling. But that is for another discussion.

This comment has been minimized.

@ruifigueira
Copy link
Contributor Author

It's failing with webkit, with the same problem I had with the other approach. For some reason, we get a click event with the body element as target and the input element as the hovered element.

A solution is to ensure the click event is handled only when the hovered element matches the event target

Copy link
Contributor

Test results for "tests 1"

4 flaky ⚠️ [chromium] › library/tracing.spec.ts:243:5 › should not include trace resources from the previous chunks
⚠️ [chromium] › library/tracing.spec.ts:243:5 › should not include trace resources from the previous chunks
⚠️ [chromium] › library/tracing.spec.ts:243:5 › should not include trace resources from the previous chunks
⚠️ [webkit] › library/tracing.spec.ts:243:5 › should not include trace resources from the previous chunks

21242 passed, 585 skipped
✔️✔️✔️

Merge workflow run.

@pavelfeldman pavelfeldman merged commit 778828c into microsoft:main Dec 31, 2023
30 checks passed
mxschmitt added a commit to mxschmitt/playwright that referenced this pull request Jan 19, 2024
mxschmitt pushed a commit that referenced this pull request Jan 19, 2024
… support (#28767)" (#29074)

This PR cherry-picks the following commits:

- e551506

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants