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

fix scrolling problem #289

Merged
merged 2 commits into from
May 10, 2024
Merged

fix scrolling problem #289

merged 2 commits into from
May 10, 2024

Conversation

LawyZheng
Copy link
Collaborator

No description provided.

@OB42
Copy link
Contributor

OB42 commented May 9, 2024

IMO using behavior:instant everywhere would probably trigger a lot more captcha/bot detectors.

@LawyZheng
Copy link
Collaborator Author

IMO using behavior:instant everywhere would probably trigger a lot more captcha/bot detectors.

Good idea. but it's more important to take screenshots than worrying about the bot-detection problem.

@OB42
Copy link
Contributor

OB42 commented May 9, 2024

IMO using behavior:instant everywhere would probably trigger a lot more captcha/bot detectors.

Good idea. but it's more important to take screenshots than worrying about the bot-detection problem.

how is scrolling related to screenshots exactly?

@wintonzheng
Copy link
Contributor

wintonzheng commented May 10, 2024

@OB42
For a long page, Skyvern does multiple scrolls until the end of the page, to generate multiple screenshots and send those to LLM together. We've tested this heavily earlier which drove us to this solution: Skyvern performs better with multiple screenshots compared to sending one big whole page screenshot - we believe it has to do with the resolution of the screenshots. Playwright's whole page screenshot resolution is not so good

Most of the websites have default auto behavior which jumps instantly when using scrollBy. We haven't seen any bot detection issues so far. Pls let us know if you find a bot detection example and we can probably help figure out a solution.

@wintonzheng
Copy link
Contributor

@LawyZheng
The scrollUpAndDown and scrollDownAndUp can be removed. It was there to counter the weird select-2 behavior which has been fixed now

@wintonzheng
Copy link
Contributor

@LawyZheng
after removing scroll up and down, we might not need the behavior: instant. Can you test that?

@LawyZheng
Copy link
Collaborator Author

@LawyZheng after removing scroll up and down, we might not need the behavior: instant. Can you test that?

sure. I will test some cases to see if it still works well without behavior: instant

@LawyZheng
Copy link
Collaborator Author

@LawyZheng after removing scroll up and down, we might not need the behavior: instant. Can you test that?

The result is negative. Still can't do scrolling if scrollUpAndDown has been removed.

@wintonzheng
Copy link
Contributor

@LawyZheng PR looks good

@LawyZheng LawyZheng force-pushed the fail-to-scroll-on-some-websites branch from 5e689cb to 0b0a143 Compare May 10, 2024 04:05
@LawyZheng LawyZheng merged commit 49baf47 into main May 10, 2024
2 checks passed
@LawyZheng LawyZheng deleted the fail-to-scroll-on-some-websites branch May 10, 2024 04:07
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.

3 participants