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.

Sign up for GitHub

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

Jump to bottom

fix: don't propagate GDK_BACKEND to subprocs #28898

Merged
merged 5 commits into from Jun 8, 2021
Merged

fix: don't propagate GDK_BACKEND to subprocs #28898

merged 5 commits into from Jun 8, 2021

Conversation

codebytere
Copy link
Member

Description of Change

Closes #28436.

GDK_BACKEND is set by Chromium as of this CL and can detrimentally affect external app behaviors, so we want to unset it and replace with the user's setting if one exists.

Checklist

Release Notes

Notes: Fixed an issue where GDK_BACKEND was being propagated to subprocesses on Linux.

@codebytere codebytere added semver/patch backwards-compatible bug fixes target/12-x-y labels Apr 28, 2021
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Apr 28, 2021
@codebytere codebytere requested a review from ckerr April 28, 2021 15:07
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Apr 29, 2021
@codebytere codebytere force-pushed the gdk-backend-remove branch 7 times, most recently from bb623c5 to 4f53a61 Compare May 4, 2021 16:42
zcbenz
zcbenz requested changes May 5, 2021
View reviewed changes
Copy link
Member

@zcbenz zcbenz left a comment •
edited

Choose a reason for hiding this comment

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

Can you also add test to check if GDK_BACKEND has actually been cleared? It is possible that Chromium decides to move the code earlier and invalidates our workaround in future.

shell/common/platform_util_linux.cc Show resolved Hide resolved
shell/browser/electron_browser_main_parts.cc Outdated Show resolved Hide resolved
@codebytere codebytere force-pushed the gdk-backend-remove branch 4 times, most recently from f530859 to cac2e3b Compare May 5, 2021 08:17
zcbenz
zcbenz reviewed May 19, 2021
View reviewed changes
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.

The code looks good to me, I think we just need a test to check if GDK_BACKEND has actually been cleared.

vially
vially reviewed May 19, 2021
View reviewed changes
shell/common/platform_util_linux.cc
@@ -122,6 +123,18 @@ bool XDGUtil(const std::vector<std::string>& argv,
// bring up a new terminal if necessary. See "man mailcap".
options.environment["MM_NOTTTY"] = "1";

// If the user set a GDK_BACKEND value of their own, use that,
// otherwise unset it becuase Chromium is setting GDK_BACKEND
Copy link
Contributor

Choose a reason for hiding this comment

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

Small typo (nitpick): becuase => because

@codebytere codebytere requested a review from zcbenz May 19, 2021 19:32
@codebytere codebytere merged commit 7b169c2 into main Jun 8, 2021
@codebytere codebytere deleted the gdk-backend-remove branch June 8, 2021 14:10
@release-clerk
Copy link

release-clerk bot commented Jun 8, 2021

Release Notes Persisted

Fixed an issue where GDK_BACKEND was being propagated to subprocesses on Linux.

@trop
Copy link
Contributor

trop bot commented Jun 8, 2021

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

@trop trop bot removed the target/12-x-y label Jun 8, 2021
@trop
Copy link
Contributor

trop bot commented Jun 8, 2021

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

@trop trop bot mentioned this pull request Jun 8, 2021
Closed
@trop
Copy link
Contributor

trop bot commented Jun 8, 2021

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

codebytere added a commit that referenced this pull request Jun 9, 2021
@codebytere
fix: don't propagate GDK_BACKEND to subprocs (#28898)
59b1f8d
@codebytere codebytere mentioned this pull request Jun 9, 2021
Open
3 tasks
@trop trop bot removed the in-flight/14-x-y label Jun 9, 2021
MarshallOfSound added a commit that referenced this pull request Jun 9, 2021
@MarshallOfSound
Revert "fix: don't propagate GDK_BACKEND to subprocs (#28898)"
dc0f6bb 
This reverts commit 7b169c2.
@MarshallOfSound MarshallOfSound mentioned this pull request Jun 9, 2021
Merged
MarshallOfSound added a commit that referenced this pull request Jun 9, 2021
@MarshallOfSound
fix: Revert "fix: don't propagate GDK_BACKEND to subprocs (#28898)" (#…
fa0323a 
…29610)

This reverts commit 7b169c2.
ckerr pushed a commit that referenced this pull request Jun 9, 2021
@MarshallOfSound @ckerr
fix: Revert "fix: don't propagate GDK_BACKEND to subprocs (#28898)" (#…
fe32002 
…29610)

This reverts commit 7b169c2.
@andersk andersk mentioned this pull request Oct 7, 2021
Open
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vially vially vially left review comments

@deepak1556 deepak1556 deepak1556 left review comments

@zcbenz zcbenz zcbenz approved these changes

@MarshallOfSound MarshallOfSound MarshallOfSound approved these changes

@ckerr ckerr Awaiting requested review from ckerr

Assignees
No one assigned
Labels
semver/patch backwards-compatible bug fixes
Projects
None yet
Milestone
No milestone
5 participants
@codebytere @vially @zcbenz @deepak1556 @MarshallOfSound