1796408 - Nightly fails to position mouse cursor to default dialog button.

Status: VERIFIED FIXED

(Core :: Widget: Win32, defect, P5)

Description

[SRovinsky]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:108.0) Gecko/20100101 Firefox/108.0

Steps to reproduce:

1. Add parameter /p to the command-line that starts Nightly.
2. Choose a bookmarks folder with a long list of bookmarks. Scroll to the bottom and select "Open All in Tabs".

Actual results:

1. The "Choose User Profile" dialog is raised. The mouse cursor remains where it was left by the user.
2. The dialog warning of a possible slowdown when opening many tabs appears. The mouse cursor is not on either button, but somewhere in the text.

Expected results:

1. The mouse cursor should have moved to the "Start Nightly" button.
2. The mouse cursor should have moved to the "Open Tabs" button.

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'Firefox::Bookmarks & History' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Stephen A Pohl [:spohl]]

(In reply to SRovinsky from comment #0)

Expected results:

1. The mouse cursor should have moved to the "Start Nightly" button.
2. The mouse cursor should have moved to the "Open Tabs" button.

Could you clarify why you expect this to occur? Do you have any accessibility settings set that make the mouse cursor jump to the default buttons when any dialog opens?

Comment 3

[SRovinsky]

The examples and guidelines for writing bugs seemed to discourage long-winded explanations, so I didn't include this part in the initial report.

I have Windows settings for the mouse set to “Automatically move pointer to the default button in a dialog box.” This setting can be reached on my system by going through Windows Settings -> Devices -> Mouse -> Additional Mouse Options (in the right margin column), which brings up an old-style Control Panel Mouse configuration dialog. Then in the Pointer Options tab, there is a checkbox with the title "Snap to" and the above legend. I have this option checked.
The mouse cursor in the Choose User Profile dialog was being repositioned into the "Start Nightly" box. When opening a long list of bookmarks in the browser, the cursor was being positioned into the "Open Tabs" box. This still works in v106, in Firefox General Availability release. It has stopped working in the new Aurora v107 release (Firefox Developer Edition), and has not worked in Nightly since the start of the v107 release, in late August or early September.

Comment 4

[Stephen A Pohl [:spohl]]

Could you run mozregression to see when exactly this started happening?

A number of Firefox versions will open in succession to narrow down when this started occurring. Simply answer "good" or "bad" based on whether or not a build reproduces the bug. Once finished, please post the output from the last run. It should give a last good and first bad revision as well as a link to look at the changesets in that range. Thank you!

Comment 5

[SRovinsky]

I ran the Mozregression, and I think it worked. I had two Good responses, and a lot of Bad responses. Here's the Log text from the last run:

2022-10-21T23:52:30.716000: INFO : Narrowed integration regression window from [f7b2ae05, 2ec56986] (3 builds) to [96677f03, 2ec56986] (2 builds) (~1 steps left)
2022-10-21T23:52:30.736000: DEBUG : Starting merge handling...
2022-10-21T23:52:30.736000: DEBUG : Using url: https://hg.mozilla.org/integration/autoland/json-pushes?changeset=2ec569867b48dc6db1cdcf5fdfd6d24c4dd221b6&full=1
2022-10-21T23:52:30.736000: DEBUG : redo: attempt 1/3
2022-10-21T23:52:30.736000: DEBUG : redo: retry: calling _default_get with args: ('https://hg.mozilla.org/integration/autoland/json-pushes?changeset=2ec569867b48dc6db1cdcf5fdfd6d24c4dd221b6&full=1',), kwargs: {}, attempt #1
2022-10-21T23:52:30.739000: DEBUG : urllib3.connectionpool: Resetting dropped connection: hg.mozilla.org
2022-10-21T23:52:31.627000: DEBUG : urllib3.connectionpool: https://hg.mozilla.org:443 "GET /integration/autoland/json-pushes?changeset=2ec569867b48dc6db1cdcf5fdfd6d24c4dd221b6&full=1 HTTP/1.1" 200 None
2022-10-21T23:52:31.651000: DEBUG : Found commit message:
Bug 1781434 - Clean-up dialog initial focus code. r=pbz,Gijs

Make it a bit easier to read and less prone to race conditions. Remove a
setTimeout referencing bug 103197 which I'm pretty sure it's not an
issue.

Differential Revision: https://phabricator.services.mozilla.com/D156543

2022-10-21T23:52:31.651000: DEBUG : Did not find a branch, checking all integration branches
2022-10-21T23:52:31.653000: INFO : The bisection is done.
2022-10-21T23:52:31.655000: INFO : Stopped

Comment 6

[Stephen A Pohl [:spohl]]

I think this was regressed by bug 1781434, not bug 103197.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1796408 - Fix windows default cursor snapping on dialogs. r=pbz,Gijs

This code used to run twice before bug 1781434: Once on load, and once
after a setTimeout().

The code didn't flush layout before load, and ran before both window sizing.
The setTimeout one ran (often at least, presumably) once layout had been made
and managed to snap the cursor properly.

Fix the ordering by ensuring it happens after window sizing and button
translation, and ensure that layout is up-to-date.

Quite an obscure windows feature, if you ask me... Also, seems disabled by
default nowadays, at least on Win11.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

This is quite the windows feature, didn't know it existed... https://www.elevenforum.com/t/turn-on-or-off-pointer-snap-to-default-button-in-windows-11.7328/

Comment 9

[SRovinsky]

Thanks for figuring this out. I kind of agree that this couldn't have been regressed by bug #103197; that bug was handled 20 years ago.
I'm also not sure that I did a good job with the mozregression. I couldn't figure out how to get the regression tool to start the test Nightly version with the "/p" command-line parameter, which is what I use on my system, and which brings up the "Choose User Profile" dialog. I tried to use the "Custom Preferences" to Add the parameter, but that had no effect. Is there a way to get it to start with a parameter? When this dialog runs, the browser hasn't started yet, and most of the conditions and data structures necessary for running what is described in these bugs hasn't even had a chance to be instantiated yet, so I'm wondering if it's the same code displaying that dialog.

Emilio: Yeah, I know, but I started out working on these boxes back when MS-DOS was the software of the day. When Windows 3.1 became popular, we were overjoyed that there still was a way to use the command line. After fighting with Windows for a while, we discovered the Control Panel, and went to town configuring everything that we could for our own convenience. It may be a little obscure, but thankfully when I upgrade from version to version of Windows, it's one of the options that gets copied over, so I don't have to do it every time.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0882e77d38ae
Fix windows default cursor snapping on dialogs. r=Gijs

Comment 11

[Marian-Vasile Laza]

Backed out changeset 0882e77d38ae (Bug 1796408) for causing bc failures on browser_sidebarpanels_click.js.
Backout link
Push with failures <--> bc8
Failure Log
Also bc6 Failure Log

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/34da030ad47d
Fix windows default cursor snapping on dialogs. r=Gijs

Comment 13

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/34da030ad47d

Comment 14

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

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox107 to wontfix.

For more information, please visit auto_nag documentation.

Comment 15

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9299750 [details]
Bug 1796408 - Fix windows default cursor snapping on dialogs. r=pbz,Gijs

Beta/Release Uplift Approval Request

• User impact if declined: comment 0
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: comment 0
• List of other uplifts needed: none
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Relatively straight-forward tweak.
• String changes made/needed: none
• Is Android affected?: No

Comment 16

[Donal Meehan [:dmeehan]]

:emilio there is a conflict with this patch and beta. Could you please attach a patch?

Comment 17

[Emilio Cobos Álvarez (:emilio)]

Attachment: Beta patch

Comment 18

[Donal Meehan [:dmeehan]]

Comment on attachment 9299750 [details]
Bug 1796408 - Fix windows default cursor snapping on dialogs. r=pbz,Gijs

Approved for 107.0b6.

Comment 19

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/7b7a6c4d0204

Comment 20

[Daniel Bodea [:danibodea]]

I've attempted verification of the latest Nightly v108.0a1 and my test results are:

1. the cursor is properly moved over the "Start Nightly" button of the Profile Manager.
   BUT
2. the cursor is not moved over the "Open tabs" button from the "Confirm open" modal, but somewhere else on the model instead.

In conclusion, I think this bug is only partially fixed in Nightly v108.0a1. What do you think, Emilio?

Comment 21

[Emilio Cobos Álvarez (:emilio)]

You're right that the position of subdialogs is busted.

Comment 22

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1796408 - Restore previous timing. r=Gijs

Big hack, but this ensures that the dialog is at the right position when it's a
subdialog positioned by the front-end... Otherwise we fire this before setting
the vertical margin in the opener, and sadness ensues.

I'd try to find a nicer solution to this, but:

• This is the easiest that is potentially upliftable.
• I'm a bit swamped with some other work.
• This is an extra one-liner, for a very niche feature.

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b26e14501201
Restore previous timing. r=Gijs

Comment 24

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/b26e14501201

Comment 25

[Daniel Bodea [:danibodea]]

I can confirm that both of the situations mentioned in comment 0 are now fixed. Verified in Nightly v108.0a1. Awaiting uplift to verify branch 107.

Comment 26

[Donal Meehan [:dmeehan]]

:emilio could you add a beta uplift request on the follow-up patch when ready?

Comment 27

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9300442 [details]
Bug 1796408 - Restore previous timing. r=Gijs

Beta/Release Uplift Approval Request

• User impact if declined: See comment 20
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: comment 0
• List of other uplifts needed: none
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Restores the pre-regression timing to fix cursor position in modal subdialogs.
• String changes made/needed: none
• Is Android affected?: No

Comment 28

[Donal Meehan [:dmeehan]]

Comment on attachment 9300442 [details]
Bug 1796408 - Restore previous timing. r=Gijs

Approved for 107.0b7.

Comment 29

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/c09c7a8560e7

Comment 30

[Daniel Bodea [:danibodea]]

This fix has also been verified on Beta v107.0b8 on Windows 10. When the respective OS customization is made, the cursor will correctly be moved to the "Start Firefox" button from the Profile Manager and the "Open Tabs" button from the "Confirm open" modal when these appear on the screen.