-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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 canonical screenshots for firefox with webdriver bidi for screenshot element tests #34289
base: main
Are you sure you want to change the base?
Add canonical screenshots for firefox with webdriver bidi for screenshot element tests #34289
Conversation
lutien
commented
Jan 10, 2025
•
edited
Loading
edited
- add canonical screenshots for Firefox with WebDriver BiDi for screenshot element tests
- add a canonical screenshot for locator test
This comment has been minimized.
This comment has been minimized.
@@ -168,7 +168,8 @@ it.describe('element screenshot', () => { | |||
|
|||
it('should work with a rotated element', async ({ page }) => { | |||
await page.setViewportSize({ width: 500, height: 500 }); | |||
await page.setContent(`<div style="position:absolute; | |||
await page.setContent(`<!DOCTYPE html> |
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.
We should not need to modify tests to make them pass. If Firefox behaves differently w/o doctype we should fix it elsewhere (prepend DOCTYPE when settings content or differently)
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.
ok, I've removed the doctype update from this PR
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.
@pavelfeldman if the doctype should be added somewhere else, do you mean directly in the page.setContent
implementation of Playwright? Quirks mode is expected to have differences in browsers, but if there are cases when you actually want to test that it won't be possible unless another argument to eg. page.setContent
is added, which would enable it as fallback.
Mind sharing what is your exact proposal here? Thanks!
…for Firefox with WebDriver BiDi.
6de4926
to
4d613b3
Compare
This comment has been minimized.
This comment has been minimized.
Test results for "tests 1"9 flaky37578 passed, 648 skipped Merge workflow run. |