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

[Bug]: Windows Control Overlay button hover effect not working #32360

Closed
3 tasks done
mlaurencin opened this issue Jan 6, 2022 · 6 comments · Fixed by #32672
Closed
3 tasks done

[Bug]: Windows Control Overlay button hover effect not working #32360

mlaurencin opened this issue Jan 6, 2022 · 6 comments · Fixed by #32672

Comments

@mlaurencin
Copy link
Contributor

Preflight Checklist

Electron Version

14.0.0

What operating system are you using?

Windows

Operating System Version

Windows 11

What arch are you using?

x64

Last Known Working Electron version

No response

Expected Behavior

See #30764 for specific behavior

Actual Behavior

See #30764 for specific behavior

Testcase Gist URL

No response

Additional Information

I am creating this issue based on #30764 and the information I have gathered while trying to triage it. Though this not a breaking bug, it does create a visual inconsistency for users, as a window control button would typically change color when hovered over. For this reason, I think it should still be addressed at some point. Consolidating all current information and future questions/comments here should help in finding a solution.

Here is a link to the bug that led to the original fix implemented in Chromium

And here is the link to the follow-up bug report created noting issues with the implemented fix. This was then labeled as WontFix

The chromium fix artificially creates the correct tooltips and allows for the button color to change on hover. However, this results in a response to the WM_NCHITTEST message that does not correspond with the function of the button. For example, HTMAXIMIZE is not returned, thus Snap Layout on Windows 11 does not work.

If someone else wants to take a look at this, the relevant Electron files can be found in this PR adding WCO to Electron. For this bug in particular, I believe the related files are: shell/browser/ui/views/win_caption_button.cc, shell/browser/ui/views/win_caption_button_container.cc, and shell/browser/ui/views/win_frame_view.cc but changes to other files may provide a solution as well.

@AngeloCore
Copy link

any update here?

@jtpotato
Copy link

jtpotato commented Jan 17, 2022

any workarounds? hope this is fixed soon

@shanselman
Copy link

👀

@clavin
Copy link
Member

clavin commented Jan 25, 2022

Just noting this observation here while I look into this: if you create a BrowserWindow but don't load anything (no loadURL or loadFile) then the window controls work perfectly fine (note: this is of a build without the first fix in chromium that breaks Snap Assist). So something about the actual web view is breaking the event flow described in the chromium bugs.

@clavin
Copy link
Member

clavin commented Jan 28, 2022

I've been diving deep into this one and I've managed to find out a lot. Take a look:

This bug seems to be triggered due to LegacyRenderWidgetHostHWND::OnMouseRange calling the view event handler twice in a very roundabout way. The first is when it calls HandleMouseMessage which eventually ends up correctly being the mouse enter event. However, the second call is when it calls the Win API function DefWindowProc, essentially causing the OS to send a WM_NCMOUSELEAVE message which is what causes the mouse exit event.

I think the only fix here is to somehow mark the original call to HandleMouseMessage as handled, which prevents LegacyRenderWidgetHostHWND::OnMouseRange from passing the message on to DefWindowProc. I can verify that this is a valid solution by manually changing Widget::OnMouseEvent to set the event as handled for any non-client event, and it does solve this bug! However, I'm not really sure if this is the right solution or not and if this could affect anything else.

# void Widget::OnMouseEvent(ui::MouseEvent* event) {
#   ...
#   case ui::ET_MOUSE_MOVED: {
#     ...
         last_mouse_event_was_move_ = true;
         if (root_view)
           root_view->OnMouseMoved(*event);
+        if ((event->flags() & ui::EF_IS_NON_CLIENT) == 0)
+          event->SetHandled();
       }
       return;
#   }
#   ...
# }

@panther7
Copy link

Hover is broken in 17.4.2 with transparent color (titleBarOverlay: { color: 'rgba(0,0,0,0') })

hover

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

Successfully merging a pull request may close this issue.

6 participants