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

Explicitly hide tooltip when the window is deactivated (1-8-x) #11629

Merged
merged 4 commits into from Jan 19, 2018

Conversation

Projects
None yet
3 participants
@brenca
Copy link
Member

brenca commented Jan 12, 2018

This is here to fix #9943

@brenca brenca requested a review from electron/reviewers as a code owner Jan 12, 2018

@brenca

This comment has been minimized.

Copy link
Member

brenca commented Jan 12, 2018

If this is an acceptable solution while the upstream issue is fixed, we could backport this to 1.7.x if needed (and I could do the same on master).

@alexeykuzmin

This comment has been minimized.

Copy link
Contributor

alexeykuzmin commented Jan 15, 2018

@brenca If the master is affected, the bug should be fixes there too.
Can you please create a PR?

aura::client::GetTooltipClient(lost_active->GetRootWindow()));

// This will cause the tooltip to be hidden when a window is deactivated,
// as it should be. Remove this fix when the chromium issue is fixed.

This comment has been minimized.

@alexeykuzmin

alexeykuzmin Jan 15, 2018

Contributor

Remove this fix when the chromium issue is fixed.

It probably needs a TODO marker.

@brenca

This comment has been minimized.

Copy link
Member

brenca commented Jan 15, 2018

Sure, gonna create a PR there too ASAP.
Update: #11644

@brenca brenca changed the title Explicitly hide tooltip when the window is deactivated Explicitly hide tooltip when the window is deactivated (1-8-x) Jan 15, 2018

@alexeykuzmin alexeykuzmin requested a review from ckerr Jan 18, 2018

@alexeykuzmin
Copy link
Contributor

alexeykuzmin left a comment

@brenca Looks like changes to master and 1-8-x are supposed to be identical,
let's update this PR after the PR to master is approved and merged.

@ckerr

ckerr approved these changes Jan 18, 2018

Copy link
Member

ckerr left a comment

With the exception of syncing wm::ActivationChangeOvserver to aura::client::, this appears to be identical to the PR for master

👍

@ckerr

This comment has been minimized.

Copy link
Member

ckerr commented Jan 19, 2018

Merging.

@alexeykuzmin Since your X was procedural, ie to wait for the sibling PR to land in master, I'm going to merge this now that the sibling's also merged. If I've misread your X please feel free to reopen both here and in master.

@ckerr ckerr merged commit 6efa330 into electron:1-8-x Jan 19, 2018

5 of 6 checks passed

continuous-integration/jenkins/pr-head This commit cannot be built
Details
ci/circleci: electron-linux-arm Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-arm64 Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-ia32 Your tests passed on CircleCI!
Details
ci/circleci: electron-linux-x64 Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment