[Windows HiDPI] search engines appearing in the incorrect row when resizing

VERIFIED FIXED in Firefox 55

Status

()

defect
P3
normal
Rank:
23
VERIFIED FIXED
5 years ago
2 years ago

People

(Reporter: kjozwiak, Assigned: florian)

Tracking

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

Firefox Tracking Flags

(firefox55 verified, firefox56 verified)

Details

(Whiteboard: [fxsearch][win10])

Attachments

(2 attachments, 2 obsolete attachments)

Reporter

Description

5 years ago
Posted image missingBorders.gif (obsolete) —
Sometimes the borders between the search engines are missing after re-sizing fx several times. I've only managed to reproduce this on my Win 7 x64 VM machine. However, I can reproduce the issue consistently after re-sizing fx a few times.

Maybe someone with a dedicated Win 7 machine (not a VM) can try and see if it's only related to the VM? (don't see how though)

STR:

1) Type something into the search toolbar
2) Once the search panel appears, click somewhere else to dismiss the panel
3) Re-size fx
4) Press down to receive the search toolbar panel once again
5) Just keep repeating #1 - #4 a few times and you'll eventually run into the issue

Attached a .gif and a .png
Reporter

Comment 1

5 years ago
Posted image missingBordersIssue.png (obsolete) —
Assignee

Comment 2

5 years ago
(In reply to Kamil Jozwiak [:kjozwiak] from comment #1)
> Created attachment 8527957 [details]
> missingBordersIssue.png

The problem here isn't missing borders: you have the last item of the first row displayed on the second row, and the last item of the second row displayed on a third row that shouldn't exist.

The problem likely happens for some specific sizes of the search box; there's likely a margin or a border that isn't correctly taken into account in the width computations.
Reporter

Comment 3

5 years ago
Reproduced using a newer build (used 34.0b11 originally):
- http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-beta-win32/1416560414/
Summary: borders between search engines disappearing when re-sizing fx → search engines appearing in the incorrect row when resizing
Assignee

Comment 4

5 years ago
The screenshot looks like it was made on a HiDPI screen. I just tried to reproduce on my Windows 7 thinkpad, but couldn't. Could this bug be HiDPI-only?
Reporter

Comment 5

5 years ago
It looks like Andrei ran into the same problem but using different STR in bug # 1104748 without a HiDPI screen. I'll try disabling "Use full resolution for Retina Display" on the VM and see if I can reproduce the problem.
Assignee

Comment 6

5 years ago
(In reply to Kamil Jozwiak [:kjozwiak] from comment #5)
> It looks like Andrei ran into the same problem but using different STR in
> bug # 1104748

His screenshot looks like a different bug to me. On your screenshot we have the correct count of items per row but the last one wraps to the next line, creating a large amount of empty space on the right side. On his screenshot there's no large whitespace on the right side.
Reporter

Comment 7

5 years ago
I couldn't reproduce the problem on the same machine once I turned off "Use full resolution for Retina Display". So it's probably related to HiDPI.

I was thinking it was a similar issue because it has some similarities when it comes to the missing borders between the search engines. Perhaps it's hitting the same margin or a border computation error but looks different as there's more search engines installed/being listed. Could be unrelated though.
Assignee

Comment 8

5 years ago
(In reply to Kamil Jozwiak [:kjozwiak] from comment #7)

> I was thinking it was a similar issue because it has some similarities when
> it comes to the missing borders between the search engines.

The bug here seems like we don't detect correctly the width of the panel, and are wrong by only a pixel or two. I fixed similar bugs before while coding this; it was typically a problem with detecting the border or margin sizes of the panel.

The problem in bug # 1104748 is that we don't support showing a large engine name at the top of a small panel, and that messes up the layout.
Assignee

Updated

5 years ago
Summary: search engines appearing in the incorrect row when resizing → [Windows HiDPI] search engines appearing in the incorrect row when resizing
Assignee

Updated

5 years ago
Duplicate of this bug: 1107493
Assignee

Comment 10

4 years ago
There's more details in bug 1187620 about which DPI sizes show the problem.
Priority: -- → P2
Whiteboard: [fxsearch]
Assignee

Updated

4 years ago
Duplicate of this bug: 1187620

Comment 12

4 years ago
(In reply to Florian Quèze [:florian] [:flo] from comment #10)
> There's more details in bug 1187620 about which DPI sizes show the problem.

Right. You don't need particular hardware - you can just set the DPI setting in Windows' control panel.

Comment 13

4 years ago
Florian has thoughts on what might be broken...
Assignee: nobody → mak77
Rank: 23
Whiteboard: [fxsearch] → [fxsearch][win10]
So, I tried reproducing this on Windows 10 with all DPI settings (125, 150, 175), insisting a little bit more on 150%, that is what most people had problems with.
After a good 40 minutes of resizing the window (large and small amounts) and testing the panel, I was still unable to reproduce any kind of rounding problem.
So, either my system is not adapt to reproduce the bug, or something else fixed some related hi-dpi stuff so now it's no more a bug.

Regardless doesn't look like I may be useful here :/
Assignee: mak77 → nobody

Comment 15

4 years ago
Posted image Current nightly
STR:

1. Use 150% DPI on current win10 insider preview (win10 regular release probably works too)
2. new profile on current nightly
3. resize window so it has outerWidth 1080 (you can check in the browser console, or use a screen that renders at 1440px width with this DPI setting (in which case that window size will be the default on Windows).

AR:
the screenshot attached

ER: correctly wrapping items. :-)

I'm bringing this machine to Orlando if that helps.
Reporter

Comment 16

4 years ago
Reproduced it as well using Win 7 x64 VM + retina following the steps from comment #15.
On my box it doesn't reproduce at all :(
Priority: P2 → P3

Updated

2 years ago
OS: Windows 7 → Windows
Hardware: x86_64 → Unspecified
Assignee

Updated

2 years ago
Attachment #8527956 - Attachment is obsolete: true
Assignee

Updated

2 years ago
Attachment #8527957 - Attachment is obsolete: true
Assignee

Updated

2 years ago
See Also: → 1316566
See Also: → 1361195
I can't reproduce with the STR in comment 15 :/
Assignee

Comment 19

2 years ago
Posted patch PatchSplinter Review
This expands the workaround added in bug 1343507 for the awesomebar panel, so that it also applies to the searchbar. I tried to find a real fix in bug 1361195 but gave up.
Attachment #8884784 - Flags: review?(adw)
Assignee

Updated

2 years ago
Assignee: nobody → florian
Status: NEW → ASSIGNED
Comment on attachment 8884784 [details] [diff] [review]
Patch

Review of attachment 8884784 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM
Attachment #8884784 - Flags: review?(adw) → review+

Comment 21

2 years ago
Pushed by florian@queze.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d3f7e63a714f
[Windows HiDPI] expand the awesomebar panel workaround to also apply to the searchbar so that one-off buttons are laid out correctly even when there are rounding issues, r=adw.

Comment 22

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d3f7e63a714f
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
Assignee

Comment 23

2 years ago
Comment on attachment 8884784 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/Bug causing the regression]: AFAIK this bug has always been there since we landed one-off buttons in the searchbar. We didn't worry much about it because back in 2014 Windows HiDPI wasn't a common configuration.
[User impact if declined]: Broken searchbar one-off buttons layout for some window sizes with some DPI settings on Windows.
[Is this code covered by automated tests?]: no.
[Has the fix been verified in Nightly?]: no.
[Needs manual test from QE? If yes, steps to reproduce]: Try to resize the window, add/remove toolbar buttons, resize the searchbar, change the Windows DPI settings, and check that the searchbar one-off buttons don't break. Also check that we didn't regress bug 1343507.
[List of other uplifts needed for the feature/fix]: Bug 1361195 (which is a regression in 55 causing the same issue but on all platforms) should be uplifted at the same time.
[Is the change risky?]: not really
[Why is the change risky/not risky?]: it's only moving an existing workaround to apply more broadly so should not be risky... but this code has been a bit fragile in the past, so I would be more confident if QE could verify.
[String changes made/needed]: none.
Attachment #8884784 - Flags: approval-mozilla-beta?
Comment on attachment 8884784 [details] [diff] [review]
Patch

Let's uplift the fix and verify in beta. 
Kamil, maybe you can take a look, or andrei's team may already have a high dpi test ongoing for beta.
Flags: needinfo?(andrei.vaida)
Attachment #8884784 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Reporter

Comment 26

2 years ago
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #24)
> Kamil, maybe you can take a look, or andrei's team may already have a high
> dpi test ongoing for beta.

I can take a look once I get back from PTO on the 16th.
Flags: needinfo?(andrei.vaida) → qe-verify+
I managed to reproduce this issue with Firefox 45.0a1 (2015-12-04), under Windows 10x64, using the STR from Comment 15.
The issue is no longer reproducible on Firefox 56.0a1 (2017-07-13), or on Firefox 55.0b9-build 2. 
Tests were performed under Windows 10x64.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.