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 a team as a code owner October 1, 2019 09:34
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Oct 1, 2019
@CezaryKulakowski
Copy link
Contributor Author

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

@jkleinsc jkleinsc requested a review from zcbenz October 1, 2019 15:08
Copy link
Member

@zcbenz zcbenz left a comment

Choose a reason for hiding this comment

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

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

@codebytere codebytere requested a review from brenca October 2, 2019 07:59
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Oct 2, 2019
Copy link
Contributor

@brenca brenca left a comment

Choose a reason for hiding this comment

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

Yepp, it looks like this is no longer needed,

@codebytere codebytere merged commit 712d77d into electron:master Oct 2, 2019
@release-clerk
Copy link

release-clerk bot commented Oct 2, 2019

Release Notes Persisted

removed unnecessary chromium patch for browser compositor on mac

@nornagon
Copy link
Member

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.

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