Closed Bug 1750487 Opened 3 years ago Closed 2 years ago

When about:preferences has page zoom other than 100%, search engine dropdown is unusable

Categories

(Firefox :: Menus, defect)

80 Branch
Unspecified
Linux
defect

Tracking

()

VERIFIED FIXED
99 Branch
Tracking Status
firefox-esr91 --- wontfix
firefox96 --- wontfix
firefox97 --- wontfix
firefox98 --- wontfix
firefox99 --- verified
firefox107 --- verified
firefox108 --- verified

People

(Reporter: yoasif, Assigned: emilio)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(2 files, 3 obsolete files)

Steps to reproduce:

  1. Navigate to about:preferences#search
  2. Increase zoom level to 120%
  3. Click the Default Search engine dropdown

What happens:

The popup doesn't appear, and it is impossible to visually select a different search engine. In some of the builds since this regression, the popup also flickers like described in bug 1726939. It has since gotten worse, however.

11:14.23 INFO: No more integration revisions, bisection finished.
11:14.23 INFO: Last good revision: 9f0fbb1431721c9eae68a3c94ae49a4d33fdb1f8
11:14.23 INFO: First bad revision: 7b82d177a6b979036f180329be6b029d690d9e0c
11:14.23 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=9f0fbb1431721c9eae68a3c94ae49a4d33fdb1f8&tochange=7b82d177a6b979036f180329be6b029d690d9e0c

Attached file about:support
Has Regression Range: --- → yes
Has STR: --- → yes
Regressed by: 1661516

Set release status flags based on info from the regressing bug 1661516

This should be easy-ish to figure out, I'd think.

Flags: needinfo?(emilio)

Store preferred rect in dev pixels, which simplifies a bit other calculations,
and ensures that the nsMenuPopupFrame code accounts for zoom etc.

Use existing conversion methods for GDK <-> Device <-> CSS conversions.

Assignee: nobody → emilio
Status: NEW → ASSIGNED
Flags: needinfo?(emilio)
Attachment #9265384 - Attachment description: Bug 1750487 - Don't update popup hierarchy if position didn't change in nsWindow::NativeMoveResizeWaylandPopup. r=stransky → Bug 1750487 - Don't update popup hierarchy if position and size didn't change in nsWindow::NativeMoveResizeWaylandPopup. r=stransky

This was also broken.

Depends on D139624

I don't see why ever mScreenRect would deal with zoom-less coordinates.
I suspect this might have been accounting for screenX/Y not dealing
with zoom in the past, but that was fixed (not long ago, actually) in
bug 1753836.

Also, browser.xhtml never has zoom. I haven't seen this causing problems
in practice in non-Wayland platforms, but I think it's the right thing
to do and the code as is is extremely confusing, since other places
applying screen rects and context menu offsets treat them as real CSS
pixels:

https://searchfox.org/mozilla-central/rev/73a8000b0c0eb527faef01ea17c6d2398622a386/layout/xul/nsMenuPopupFrame.cpp#2407-2411

So I don't think this changes behavior in practice in non-wayland
platforms given we never have full zoom in browser.xhtml and we mostly
hit this code path for context menus, but given the above I think it's
simpler and more correct.

Depends on D139670

Blocks: 1757106
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a2eceb0d288a
Fix weird full-zoom handling for unanchored popups in nsMenuPopupFrame positioning. r=mstange

Comment on attachment 9265383 [details]
Bug 1750487 - Clean up menu popup rect handling. r=stransky

Revision D139623 was moved to bug 1757106. Setting attachment 9265383 [details] to obsolete.

Attachment #9265383 - Attachment is obsolete: true

Comment on attachment 9265384 [details]
Bug 1750487 - Don't update popup hierarchy if position and size didn't change in nsWindow::NativeMoveResizeWaylandPopup. r=stransky

Revision D139624 was moved to bug 1757106. Setting attachment 9265384 [details] to obsolete.

Attachment #9265384 - Attachment is obsolete: true

Comment on attachment 9265426 [details]
Bug 1750487 - Fix early-out in nsMenuPopupFrame::MoveTo to account for margins. r=stransky

Revision D139670 was moved to bug 1757106. Setting attachment 9265426 [details] to obsolete.

Attachment #9265426 - Attachment is obsolete: true
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/09583277ae5e
Fix test_popup_scaled to account for intentional behavior change.
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/2543c933025b
Avoid unneeded zoom mess-up calculation now that screen coordinates are correct. rpending=nchevobbe

(In reply to Pulsebot from comment #14)

Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/2543c933025b
Avoid unneeded zoom mess-up calculation now that screen coordinates are
correct. rpending=nchevobbe

Nicolas, I had to push this to avoid backout because there was a devtools test failure (see commit message).

I decided to push it because I'm fairly sure it's the right fix (plus it's simpler which is always nice). Assuming this sticks, can you take a quick look?

Flags: needinfo?(nchevobbe)
Pushed by ctuns@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4540f58e953c
Tentatively make test_popup_scaled's last test work on Windows on automation. CLOSED TREE

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

(In reply to Pulsebot from comment #14)

Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/2543c933025b
Avoid unneeded zoom mess-up calculation now that screen coordinates are
correct. rpending=nchevobbe

Nicolas, I had to push this to avoid backout because there was a devtools test failure (see commit message).

I decided to push it because I'm fairly sure it's the right fix (plus it's simpler which is always nice). Assuming this sticks, can you take a quick look?

Thanks Emilio, the change makes sense to me

Flags: needinfo?(nchevobbe)
Component: Widget: Gtk → Menus
Product: Core → Firefox

For the record, this made the menus usable, but bug 1757106 will actually fix their position in presence of zoom.

Flags: qe-verify+

I tried to reproduce this issue on Firefox 98, on Linux x86_64(Ubuntu 20.04) with no success. Are there any additional steps/configuration details i should follow?
If not, would you be kind to verify this fix on the latest beta? https://archive.mozilla.org/pub/firefox/candidates/99.0b4-candidates/build1/win64/en-US/
Thank you.

Flags: needinfo?(emilio)

You need MOZ_ENABLE_WAYLAND=1, probably.

Flags: needinfo?(emilio)

Reproducible with Firefox 98.0(build ID: 20220304153049) on Ubuntu 22(Wayland Session). Verified as fixed on Firefox 99.0(build ID: 20220330194208), Firefox 107.0b4(build ID: 20221023190001) and Nightly 108.0a1(build ID: 20221024212806) on Ubuntu 22.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: