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: remove unnecessary patch #20390

Conversation

@CezaryKulakowski
Copy link
Contributor

@CezaryKulakowski CezaryKulakowski commented Oct 1, 2019

Patch which this pr removes adds new version of
BrowserCompositorMac::GetCompositor. There is this function in
BrowserCompositorMac with specifier const. Chromium's function
returns parent's compositor if parent is set. Electron's version
doesn't and - as a result - constrained dialogs won't be displayed
correctly if they don't have its own compositor.

Notes: removed unnecessary chromium patch for browser compositor on mac

Patch which this pr removes adds new version of
BrowserCompositorMac::GetCompositor. There is this function in
BrowserCompositorMac with specifier const. Chromium's function
returns parent's compositor if parent is set. Electron's version
doesn't and - as a result - constrained dialogs won't be displayed
correctly if they don't have its own compositor.
@CezaryKulakowski CezaryKulakowski requested a review from as a code owner Oct 1, 2019
@CezaryKulakowski
Copy link
Contributor Author

@CezaryKulakowski CezaryKulakowski commented Oct 1, 2019

Patch which this pr removes was created by Cheng Zhao so he should know if removing it now is safe.

Loading

@jkleinsc jkleinsc requested a review from zcbenz Oct 1, 2019
zcbenz
zcbenz approved these changes Oct 2, 2019
Copy link
Member

@zcbenz zcbenz left a comment

This patch was added for supporting offscreen, it seems no longer needed, but I would like have confirmation from @brenca .

Loading

@codebytere codebytere requested a review from brenca Oct 2, 2019
brenca
brenca approved these changes Oct 2, 2019
Copy link
Contributor

@brenca brenca left a comment

Yepp, it looks like this is no longer needed,

Loading

@codebytere codebytere merged commit 712d77d into electron:master Oct 2, 2019
8 of 9 checks passed
Loading
@release-clerk
Copy link

@release-clerk release-clerk bot commented Oct 2, 2019

Release Notes Persisted

removed unnecessary chromium patch for browser compositor on mac

Loading

@nornagon
Copy link
Member

@nornagon nornagon commented Oct 2, 2019

@reviewers, please review release notes before merging a PR! Our checklist says:

  • PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.

The notes in this PR do not meet those criteria.

Loading

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

Successfully merging this pull request may close these issues.

None yet

5 participants