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: html fullscreen transitions stacking #34908
Conversation
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.
There are some merge conflicts, so this needs a rebase.
5f2ec00
to
2ce343c
Compare
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.
The test failures look unrelated, LGTM.
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.
Appears to be the same errors we saw in #34909, except now it happens on mas-testing-x64-tests
instead of osx-testing-x64-tests
.
const width = await w.webContents.executeJavaScript( | ||
"document.querySelector('iframe').offsetWidth" | ||
); | ||
expect(width).to.equal(0); |
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.
This assertion is failing on macOS
not ok 1119 iframe using HTML fullscreen API while window is OS-fullscreened can fullscreen from out-of-process iframes (macOS)
AssertionError: expected 1024 to equal 0
at Context.<anonymous> (electron/spec-main/chromium-spec.ts:1701:22)
// TODO(jkleinsc) fix this flaky test on WOA | ||
ifit(process.platform !== 'win32' || process.arch !== 'arm64')('can fullscreen from in-process iframes', async () => { | ||
if (process.platform === 'darwin') await emittedOnce(w, 'enter-full-screen'); |
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.
And this one times out
not ok 1120 iframe using HTML fullscreen API while window is OS-fullscreened can fullscreen from in-process iframes
Error: Timeout of 30000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/Users/distiller/project/src/electron/spec-main/chromium-spec.ts)
at listOnTimeout (node:internal/timers:559:17)
at processTimers (node:internal/timers:502:7)
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.
yes, these appear to be flakes as they show up on other PRs too. I've rerun.
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.
hmm, there are some more unrelated errors now but the errors I shared here are still present. Do you have a link to an old CI run in the 19-x-y line that has the same error?
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.
LGTM
Release Notes Persisted
|
Backport of #32905.
See that PR for details.
Notes: Fixes an issue with fullscreen transitions when HTML fullscreen is requested.