Closed Bug 1104325 Opened 10 years ago Closed 7 years ago

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

Categories

(Firefox :: Search, defect, P3)

Unspecified
Windows
defect

Tracking

()

VERIFIED FIXED
Firefox 56
Tracking Status
firefox55 --- verified
firefox56 --- verified

People

(Reporter: kjozwiak, Assigned: florian)

References

Details

(Whiteboard: [fxsearch][win10])

Attachments

(2 files, 2 obsolete files)

Attached 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
Attached image missingBordersIssue.png (obsolete) —
(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.
Summary: borders between search engines disappearing when re-sizing fx → search engines appearing in the incorrect row when resizing
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?
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.
(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.
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.
(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.
Summary: search engines appearing in the incorrect row when resizing → [Windows HiDPI] search engines appearing in the incorrect row when resizing
There's more details in bug 1187620 about which DPI sizes show the problem.
Priority: -- → P2
Whiteboard: [fxsearch]
(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.
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
Attached 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.
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
OS: Windows 7 → Windows
Hardware: x86_64 → Unspecified
Attachment #8527956 - Attachment is obsolete: true
Attachment #8527957 - Attachment is obsolete: true
See Also: → 1316566
See Also: → 1361195
I can't reproduce with the STR in comment 15 :/
Attached 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: 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+
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.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
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+
(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.

Attachment

General

Created:
Updated:
Size: