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: html fullscreen transitions stacking #34908

Merged
merged 1 commit into from Jul 27, 2022

Conversation

codebytere
Copy link
Member

Backport of #32905.

See that PR for details.

Notes: Fixes an issue with fullscreen transitions when HTML fullscreen is requested.

@codebytere codebytere requested a review from a team July 13, 2022 16:38
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Jul 13, 2022
@trop trop bot mentioned this pull request Jul 13, 2022
4 tasks
@trop trop bot added 19-x-y backport This is a backport PR semver/patch backwards-compatible bug fixes labels Jul 13, 2022
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Jul 13, 2022
Copy link
Member

@RaisinTen RaisinTen left a 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.

@codebytere codebytere force-pushed the fix-fs-transitions-for-chrome-fs-19 branch from 5f2ec00 to 2ce343c Compare July 14, 2022 09:35
RaisinTen
RaisinTen previously approved these changes Jul 14, 2022
Copy link
Member

@RaisinTen RaisinTen left a 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.

Copy link
Member

@RaisinTen RaisinTen left a 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);
Copy link
Member

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');
Copy link
Member

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)

Copy link
Member Author

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.

Copy link
Member

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?

@RaisinTen RaisinTen dismissed their stale review July 15, 2022 11:59

related test failures

Copy link
Member

@RaisinTen RaisinTen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@codebytere codebytere merged commit 438347d into 19-x-y Jul 27, 2022
@codebytere codebytere deleted the fix-fs-transitions-for-chrome-fs-19 branch July 27, 2022 14:24
@release-clerk
Copy link

release-clerk bot commented Jul 27, 2022

Release Notes Persisted

Fixes an issue with fullscreen transitions when HTML fullscreen is requested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
19-x-y backport This is a backport PR semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants