The default bug view has changed. See this FAQ.

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

VERIFIED FIXED in Firefox 52

Status

()

Firefox
Location Bar
P3
normal
VERIFIED FIXED
5 months ago
2 days ago

People

(Reporter: quicksaver, Assigned: adw)

Tracking

unspecified
Firefox 54
Unspecified
Windows
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox52 verified, firefox53 fixed, firefox54 verified, firefox-esr52 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

Created attachment 8809367 [details]
Search settings button in its own line in location bar

See screenshot.

STR: change "layout.css.devPixelsPerPx" to anything between 1.325 and 1.965 (Windows can set 150% zoom factor easily == 1.5)

Updated

5 months ago
Component: Search → Location Bar
Priority: -- → P3

Comment 1

4 months ago
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)

Updated

2 months ago
Assignee: nobody → adw
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
(Assignee)

Comment 3

2 months ago
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. :-/
(Assignee)

Comment 4

2 months ago
(which works BTW)

Comment 5

2 months ago
mozreview-review
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: → bug 1104325

Comment 6

2 months ago
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

Comment 7

2 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8ef05c211734
Status: ASSIGNED → RESOLVED
Last Resolved: 2 months ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54

Comment 8

2 months ago
Looks like this may have fixed bug 1325816 too?
Blocks: 1325816
Flags: needinfo?(adw)

Comment 9

a month ago
(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.
(Assignee)

Comment 11

a month ago
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.
(Assignee)

Comment 13

a month ago
OK.  This isn't a problem in the searchbar though because only the urlbar uses the "compact" settings button with the gear icon.
(Assignee)

Comment 14

a month ago
Created attachment 8836144 [details] [diff] [review]
Aurora and Beta patch

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?

Updated

a month ago
status-firefox52: --- → affected
status-firefox53: --- → affected
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+
https://hg.mozilla.org/releases/mozilla-aurora/rev/1acbb1168e116fd67dacbd2f740f4c28cffc8a05
status-firefox53: affected → fixed
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+

Comment 18

a month ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/6662ad2a09b6
status-firefox52: affected → fixed
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

Comment 20

a month ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-esr52/rev/6662ad2a09b6
status-firefox-esr52: --- → fixed
(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)
(Assignee)

Comment 22

24 days ago
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)
(Assignee)

Comment 23

24 days ago
Oh great, it seems to depend on the window width. :-P  I'll clone a new bug.
(Assignee)

Updated

24 days ago
Blocks: 1343507

Comment 24

23 days ago
[bugday-20170301]
status-firefox52: fixed → verified
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
status-firefox54: fixed → verified
You need to log in before you can comment on or make changes to this bug.