1892835 - Try to remove the delayed ncactivate hack

Status: ASSIGNED

(Core :: Widget: Win32, task, P3)

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:rkraesig, could you have a look please?

For more information, please visit BugBot documentation.

Comment 2

[Ray Kraesig [:rkraesig]]

Kind of tricky without a description.

:emilio: mind editing comment 0 to include a few words, possibly accompanied by a searchfox link?

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Err, yeah, I think I wrote a patch for this, but I needed to test it more...

This is about this code and related code from bug 1399126 and bug 953146, which seemed to come from odd touchpad drivers calling SetForegroundWindow on a popup... We have mousewheel.debug.make_window_under_cursor_foreground to emulate this behavior.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

(Removing the ni? since it's relatively low priority, I need to deal with other stuff first...)

Comment 5

[Ray Kraesig [:rkraesig]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)

(Removing the ni? since it's relatively low priority, I need to deal with other stuff first...)

That's good enough for me!

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #3)

Err, yeah, I think I wrote a patch for this, but I needed to test it more...

This is about this code and related code from bug 1399126 and bug 953146, which seemed to come from odd touchpad drivers calling SetForegroundWindow on a popup...

Exactly. When I bought first generation ASUS ZenBook, their custom touchpad driver made window under the cursor forcibly activate even if the window is a popup. Then, our window/focus management was confused because such tricky behavior was not expected. Therefore, I added the hack.

We have mousewheel.debug.make_window_under_cursor_foreground to emulate this behavior.

Yes. I wonder, do you find some problems with the hack? Otherwise, I don't think we should remove the hack because on Windows, some touchpad/mouse drivers change their behavior from the window class name under the mouse cursor or focused window class name (or rarely process name). Therefore, even if Chrome does not have the hack, removing it is risky if we don't have any problems.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Yes. I wonder, do you find some problems with the hack?

Yes, I was struggling with this because for top levels semi-transparent windows we need to deal with NCACTIVATE sync (as otherwise DWM would draw a titlebar on our back). So removing this seemed one nice way of simplifying the set up.

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #7)

Yes. I wonder, do you find some problems with the hack?

Yes, I was struggling with this because for top levels semi-transparent windows we need to deal with NCACTIVATE sync (as otherwise DWM would draw a titlebar on our back). So removing this seemed one nice way of simplifying the set up.

Oh, is it a new feature or something? I'd like you to point the bug# in see also or depends on field.

Well, the original fix has 2 changes, one is automatically refocusing the parent window of the popup and preventing to close the popup. The other is preventing to flicker the non-client area painting by the temporary focus move. I wonder, only dropping the latter hack for the painting issue would be okay. (I guess and hope that such bad drivers have gone especially after Win10/11 started to send wheel messages to the window under the cursor instead of focused window.)

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Yeah, wdyt of trying to disable it with two different prefs? Then hopefully we can remove the code soon enough as-is, otherwise we need to think of something else.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Well, I don't think that the most victims of the bad touchpad drivers would find the pref for fixing "regression" by themselves. Therefore, I guess it's fine to drop the NCACTIVE hack completely, but you'd like to keep the hack behind a pref, it's also fine to me. Up to you.

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Yeah, this was mostly about being able to roll it back if needed, but it's early in the cycle so let's try to remove it altogether. Also nowadays our style invalidation story for window activeness changes is much better, so it shouldn't cause such badness as before.

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1892835 - Remove delayed NCACTIVATE hack. r=masayuki!,#win-reviewers

See comment in the bug.

This was needed for bogus mouse drivers back in the XP/Vista era.
However Win10/11 sends wheel messages to the window under the cursor
instead of focused window, so these kinds of issues shouldn't arise
there. Try to remove this legacy hack.