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: ensure ready-to-show event is fired #25632

Merged
merged 4 commits into from Sep 28, 2020

Conversation

CezaryKulakowski
Copy link
Contributor

Description of Change

When this chromium change landed:
https://chromium-review.googlesource.com/c/chromium/src/+/2215143
event ready-to-show is not always emitted. This pull request removes following workaround for the problem:
#25448
and fixes the problem the way which ensures that event is emitted exactly at the same moment it would be emitted before problematic chromium change landed. Previous fix was not ideal as with it event ready-to-show is emitted only after did-finish-load was received what significantly delays moment the window is displayed (also: it may happen that did-finish-load is never emitted for certain pages).

Checklist

Release Notes

Notes: Fix ready-to-show event not emitted on some machines.

When this chromium change landed:
https://chromium-review.googlesource.com/c/chromium/src/+/2215143
event `ready-to-show` is not always emitted. This change ensures
that it will be fired exactly at the same moment it was being fired
before problematic chromium change landed.
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Sep 25, 2020
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Sep 26, 2020
@nornagon nornagon merged commit b85195e into electron:master Sep 28, 2020
@release-clerk
Copy link

release-clerk bot commented Sep 28, 2020

Release Notes Persisted

Fix ready-to-show event not emitted on some machines.

@trop
Copy link
Contributor

trop bot commented Sep 28, 2020

I was unable to backport this PR to "10-x-y" cleanly;
you will need to perform this backport manually.

@trop
Copy link
Contributor

trop bot commented Sep 28, 2020

I was unable to backport this PR to "9-x-y" cleanly;
you will need to perform this backport manually.

@trop
Copy link
Contributor

trop bot commented Sep 28, 2020

I have automatically backported this PR to "11-x-y", please check out #25672

@trop
Copy link
Contributor

trop bot commented Oct 13, 2020

@codebytere has manually backported this PR to "10-x-y", please check out #25932

@trop
Copy link
Contributor

trop bot commented Oct 13, 2020

@codebytere has manually backported this PR to "9-x-y", please check out #25933

@pushkin-
Copy link

pushkin- commented Jan 8, 2021

@CezaryKulakowski Which version of 11 did this land in? 11.0.0? I'm not seeing this in the release notes, only the notes for this PR.

And I have case where ready-to-show isn't firing when I load a page into a BrowserView in the BrowserWindow, but don't load a page in the BrowserWindow itself. Is this what the bug is fixing, or no?

actually, it fires on my machine, but not on others when loading specific pages in the BrowserView (possibly pages that are load balanced)

Maybe OnFirstNonEmptyLayout isn't running?

@pushkin-
Copy link

@CezaryKulakowski @codebytere this PR seems to have re-introduced this issue

@killghost
Copy link

This bug still exists in electron 13, when can it be fixed? grateful

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants