-
Notifications
You must be signed in to change notification settings - Fork 32
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
tests: add more screenshot tests #1713
base: main
Are you sure you want to change the base?
Conversation
f34365b
to
d9db826
Compare
Looks like tests fail |
098a8a2
to
8b2e2af
Compare
@@ -771,12 +771,12 @@ export class BrowsingContextImpl { | |||
switch (params.origin) { | |||
case 'document': { | |||
script = String(() => { | |||
const element = document.documentElement; | |||
const rect = document.body.getBoundingClientRect(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would not it make the problem worse meaning given that the body can be made of size 0x0 while the content is still visible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, that's a good argument.
8b2e2af
to
0e33fed
Compare
@sadym-chromium assert_screenshot looks useful, let's revert the mapper changes though. WDYT? |
agree |
Adds a screenshot fixture and adds more test.