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

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P3)

Description

[Luís Miguel [:quicksaver] (not reading bugmail)]

Attachment: 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)

Comment 1

[ovidiu boca[:Ovidiu]]

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.

Comment 2

[Drew Willcoxon :adw]

Attachment: Last Comment Bug 1316566 - Search settings button moves to its own line with 150% zoom.

Review commit: https://reviewboard.mozilla.org/r/109802/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/109802/

Comment 3

[Drew Willcoxon :adw]

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. :-/

Comment 4

[Drew Willcoxon :adw]

(which works BTW)

Comment 5

[Florian Quèze [:florian]]

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.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
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

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8ef05c211734

Comment 8

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Looks like this may have fixed bug 1325816 too?

Comment 9

[Itiel]

(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.

Comment 10

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

It would be nice to uplift the fix, it's more common nowadays on Win10 to use non default dpi settings.

Comment 11

[Drew Willcoxon :adw]

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.

Comment 12

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

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.

Comment 13

[Drew Willcoxon :adw]

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

Comment 14

[Drew Willcoxon :adw]

Attachment: 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

Comment 15

[Gerry Chang [:gchang]]

Comment on attachment 8836144 [details] [diff] [review]
Aurora and Beta patch

Fix a UI issue related to search. Aurora53+.

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1acbb1168e116fd67dacbd2f740f4c28cffc8a05

Comment 17

[Julien Cristau [:jcristau]]

Comment on attachment 8836144 [details] [diff] [review]
Aurora and Beta patch

ui fix for search in compact mode, beta52+

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/6662ad2a09b6

Comment 19

[roxleitan]

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

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/6662ad2a09b6

Comment 21

[roxleitan]

(In reply to [e-mail] 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?

Comment 22

[Drew Willcoxon :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.

Comment 23

[Drew Willcoxon :adw]

Oh great, it seems to depend on the window width. :-P  I'll clone a new bug.

Comment 24

[suba4598]

[bugday-20170301]

Comment 25

[Andrei Vaida [:avaida]]

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).

Comment 26

[roxleitan]

Based on comment 19 from this bug and comment 8 from bug 1343507 I will mark this as Verified Fixed.

Comment 27

[Surentharan]

[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.