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

Require !OS_MACOSX as well as TOOLKIT_VIEWS for views code #12884

Merged
merged 2 commits into from May 10, 2018

Conversation

Projects
None yet
2 participants
@nornagon
Copy link
Contributor

nornagon commented May 10, 2018

The GN build defines TOOLKIT_VIEWS even on macOS, but some code in Electron guarded by TOOLKIT_VIEWS expects to be only built on Linux and Windows. This PR adds extra guards to that code.

@nornagon nornagon requested a review from as a code owner May 10, 2018

@nornagon nornagon referenced this pull request May 10, 2018

Merged

GN build #12837

@nornagon nornagon requested a review from alexeykuzmin May 10, 2018

@alexeykuzmin

This comment has been minimized.

Copy link
Contributor

alexeykuzmin commented May 10, 2018

Require OS_MACOSX as well as TOOLKIT_VIEWS for views code

-#if defined(TOOLKIT_VIEWS)
+#if defined(TOOLKIT_VIEWS) && !defined(OS_MACOSX)
   HideAutofillPopup();
 #endif

The title and the changes say quite opposite things =)

@nornagon nornagon changed the title Require OS_MACOSX as well as TOOLKIT_VIEWS for views code Require !OS_MACOSX as well as TOOLKIT_VIEWS for views code May 10, 2018

@nornagon

This comment has been minimized.

Copy link
Contributor Author

nornagon commented May 10, 2018

Whoops, good catch. fixed the PR title.

@nornagon nornagon force-pushed the nornagon:views-defines branch from 8d955a0 to 72057bf May 10, 2018

@nornagon nornagon merged commit cc386f2 into electron:master May 10, 2018

1 of 8 checks passed

ci/circleci: electron-linux-arm CircleCI is running your tests
Details
ci/circleci: electron-linux-arm64 CircleCI is running your tests
Details
ci/circleci: electron-linux-ia32 CircleCI is running your tests
Details
ci/circleci: electron-linux-x64 CircleCI is running your tests
Details
ci/circleci: electron-mas-x64 CircleCI is running your tests
Details
ci/circleci: electron-osx-x64 CircleCI is running your tests
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
WIP ready for review
Details

@nornagon nornagon deleted the nornagon:views-defines branch May 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.