Closed Bug 1316566 Opened 3 years ago Closed 3 years ago

Search settings button moves to its own line with 150% zoom

Categories

(Firefox :: Address Bar, defect, P3)

Unspecified
Windows
defect

Tracking

()

VERIFIED FIXED
Firefox 54
Tracking Status
firefox52 --- verified
firefox-esr52 --- fixed
firefox53 --- fixed
firefox54 --- verified

People

(Reporter: quicksaver, Assigned: adw)

References

Details

Attachments

(3 files)

See screenshot.

STR: change "layout.css.devPixelsPerPx" to anything between 1.325 and 1.965 (Windows can set 150% zoom factor easily == 1.5)
Component: Search → Location Bar
Priority: -- → P3
Hi, 
I can reproduce this on Windows 7 x64 with the latest Nightly 52.0a1(2016-11-13). I tried to reproduce this on Mac OS X 10.10 but I can't.
Assignee: nobody → adw
Status: NEW → ASSIGNED
To recap, the width of the one-off buttons is integral, which means that when the popup width is not a multiple of the button width, there's some remainder that's left over at the right edge of the popup.  When there's a single row of one-offs, we make the final dummy button wider than all the other buttons by this remainder so that the settings button hugs the right edge of the popup.

I can't explain what's going on in this case, but the width of that final dummy button is somehow ~1px too big, which pushes the settings button onto a new row.  It must have something to do with layout pixels not being an integral multiple of the device pixels -- clearly.  But I don't know what.

Since I don't exactly understand the problem, I'm not sure what the right fix is.  Seems like a layout bug to me.  This patch just unscales the remainder, floors it, scales it back, and then floors that. :-/
(which works BTW)
Comment on attachment 8833578 [details]
Last Comment Bug 1316566 - Search settings button moves to its own line with 150% zoom.

https://reviewboard.mozilla.org/r/109802/#review111958

That seems reasonable; and thanks for including a comment that's detailed enough to ensure the next person looking at this will understand what we are doing :-).

I guess we'll need to do something similar for bug 1104325.
Attachment #8833578 - Flags: review?(florian) → review+
See Also: → 1104325
Pushed by dwillcoxon@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8ef05c211734
Last Comment Bug 1316566 - Search settings button moves to its own line with 150% zoom. r=florian
https://hg.mozilla.org/mozilla-central/rev/8ef05c211734
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
Looks like this may have fixed bug 1325816 too?
Blocks: 1325816
Flags: needinfo?(adw)
(In reply to Marco Bonardo [::mak] from comment #8)
> Looks like this may have fixed bug 1325816 too?

Yep, I can confirm this is fixed on latest Nightly.
It would be nice to uplift the fix, it's more common nowadays on Win10 to use non default dpi settings.
urlbar one-offs are disabled on non-Nightly though, so I'm not sure it's worth uplifting.  But I'll request it if you still think so.
Flags: needinfo?(adw)
I think it's worth to fix the searchbar, plus if we run another Shield on the new urlbar it would be better to not get a broken experience.
OK.  This isn't a problem in the searchbar though because only the urlbar uses the "compact" settings button with the gear icon.
Approval Request Comment
[Feature/Bug causing the regression]: Bug 1180944
[User impact if declined]: This bug
[Is this code covered by automated tests?]: This code, yes, this bug, no
[Has the fix been verified in Nightly?]: Not "officially" but commenters here have verified it
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Tiny patch specific to a tiny part of the code
[String changes made/needed]: None
Attachment #8836144 - Flags: approval-mozilla-beta?
Attachment #8836144 - Flags: approval-mozilla-aurora?
Comment on attachment 8836144 [details] [diff] [review]
Aurora and Beta patch

Fix a UI issue related to search. Aurora53+.
Attachment #8836144 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8836144 [details] [diff] [review]
Aurora and Beta patch

ui fix for search in compact mode, beta52+
Attachment #8836144 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
Verified as fixed using the latest Nightly 54.0a1(Build ID:20170216030210) on Windows 10 x64, with "layout.css.devPixelsPerPx" set to 1.5

The issue is still reproducible on Windows 7 x86 with latest Nightly 54.0a1(Build ID:20170216030210) and Ubuntu 16.04 with Nightly 54.0a1 (Build ID:20170217110156) with "layout.css.devPixelsPerPx" set to 1.5
(In reply to roxana.leitan@softvision.ro from comment #19)
 
> The issue is still reproducible on Windows 7 x86 with latest Nightly
> 54.0a1(Build ID:20170216030210) and Ubuntu 16.04 with Nightly 54.0a1 (Build
> ID:20170217110156) with "layout.css.devPixelsPerPx" set to 1.5

Drew, any thoughts on this?
Flags: needinfo?(adw)
Strange, I can reproduce the problem too on Nightly but not on my own build.  I'll try to find out what's going on.
Flags: needinfo?(adw)
Oh great, it seems to depend on the window width. :-P  I'll clone a new bug.
Blocks: 1343507
[bugday-20170301]
Removing the qe-verify flag since there's nothing else to be done here and a follow up bug (Bug 1343507) has been filed separately for Roxana's concerns (see Comment 19).
Flags: qe-verify+
Based on comment 19 from this bug and comment 8 from bug 1343507 I will mark this as Verified Fixed.
Status: RESOLVED → VERIFIED
[Bugday-20170405]
OS :  Windows 8.1
Browser : Firefox 54.0b3 (64-bit)
I tried to reproduce the bug and the bug has been fixed and verified.
You need to log in before you can comment on or make changes to this bug.