Repository navigation
feat(browserContext): support maximized viewport - #43363
Devin Rousso (dcrousso) wants to merge 2 commits into
Conversation
Test results for "MCP"2 failed 9202 passed, 1495 skipped Merge workflow run. |
|
Hi, I'm the Playwright bot and I took a first look at the CI failures. 🟢 Both failures are confirmed pre-existing flakesThe same tests have failed on Windows/Firefox on revisions unrelated to this PR. This PR only activates its new window behavior when DetailsOverall assessment The two MCP failures are not likely caused by this PR. The other four reported tests passed on retry, so they do not require failure triage. Pre-existing flakes
Triaged by the Playwright bot - agent run |
| Use `null` to disable the consistent viewport emulation. Learn more about [viewport emulation](../emulation#viewport). | ||
| Use `"maximized"` to disable viewport emulation and maximize each native browser window. This is only supported in headed mode and does not enter fullscreen mode or trigger the web Fullscreen API. | ||
|
|
||
| Calling [`method: Page.setViewportSize`] overrides `"maximized"` for that page and resizes its native window. |
There was a problem hiding this comment.
I think this sentence is confusing rather than helping.
| if (options.userAgent) | ||
| lines.push(` .setUserAgent(${quote(options.userAgent)})`); | ||
| if (options.viewport) | ||
| if (options.viewport && options.viewport !== 'maximized') |
There was a problem hiding this comment.
Why suddenly a java-only change? I'd expect either all languages or none.
| tests non-deterministic. | ||
| ::: | ||
|
|
||
| ## java-context-option-viewport |
There was a problem hiding this comment.
Let's sketch out how this API looks for other languages right away.
| properties: | ||
| width: int | ||
| height: int | ||
| viewportMaximized: boolean? |
There was a problem hiding this comment.
I'd recommend an enum viewportMode instead of noDefaultViewport+viewportMaximized. Otherwise it's unclear what noDefaultViewport: true, viewportMaximized: true means.
| it('should create context in maximized window without entering fullscreen', async ({ browser, browserName }) => { | ||
| const context = await browser.newContext({ viewport: 'maximized' }); | ||
| const page = await context.newPage(); | ||
| await expect.poll(() => windowIsMaximized(context, page, browserName)).toBe(true); |
There was a problem hiding this comment.
Why do we have to poll here? Shouldn't the newPage already be maximized right away? I'd expect that from the API.
| startAutomaticVideoRecording(this._page); | ||
| await Promise.all(promises); | ||
| if (contextOptions.viewportMaximized) { | ||
| await this._browserContext._browser._browserSession.send('Playwright.maximizeWindow', { |
There was a problem hiding this comment.
- Why is this not a part of
_updateViewport()method? It seems to be either viewport or maximized, but not both. - Similarly, does setting viewport undo
Playwright.maximizeWindow? If so, perhaps it should be the same webkit protocol method as well?
| await context.close(); | ||
| }); | ||
|
|
||
| it('should override maximized viewport when resizing a page', async ({ browser, browserName }) => { |
There was a problem hiding this comment.
I'd expect a symmetric test as well: create a context with viewport=567x345, then call page.setViewportSize('maximized') after.
| }, | ||
| })); | ||
| } | ||
| if (this._options.viewportMaximized) { |
There was a problem hiding this comment.
It feels like this should be a part of doUpdateDefaultViewport().
|
|
||
| async _initialize(hasUIWindow: boolean) { | ||
| const browserOptions = this._crPage._browserContext._browser.options; | ||
| const options = this._crPage._browserContext._options; |
There was a problem hiding this comment.
nit: contextOptions
There was a problem hiding this comment.
I'd expect the new "maximized" option to affect metricsOverride somehow...
allow `viewport: 'maximized'` to maximize native browser windows without a default emulated viewport keep native maximization independent of browser fullscreen and the web Fullscreen API preserve the maximized outer window when `Page.setViewportSize()` enables viewport emulation
`noDefaultViewport` duplicates `viewportMode` use `viewportMode` as the only viewport mode setting
12ac87f to
86ffb77
Compare
This branch has not been deployed
2 participants
allow
viewport: 'maximized'to maximize native browser windows without a default emulated viewportkeep native maximization independent of browser fullscreen and the web Fullscreen API
preserve the maximized outer window when
Page.setViewportSize()enables viewport emulationfixes #1086