1773052 - The last entry in the bookmark is partially invisible, and the bookmark scroll button does not work

Status: VERIFIED FIXED

(Toolkit :: XUL Widgets, defect)

Description

[Mayank Bansal]

Create fresh profile
Using the attached json file, "restore" the bookmarks
Add the bookmark button to the Toolbar
Click on the bookmark button, and scroll to hte bottom of the folder

ER: the last bookmark is partially obscured, and the bookmark scroll button does not work
AR: Not so

Comment 1

[Mayank Bansal]

Attachment: bookmarks-2022-06-07.json

Comment 2

[Mayank Bansal]

Attachment: placement of bookmark button.png

Comment 3

[Mayank Bansal]

Attachment: bandicam 2022-06-07 20-44-24-679.mp4

Comment 4

[Mayank Bansal]

2022-06-07T20:22:02.334000: DEBUG : Found commit message:
Bug 1768278 - Draw menupopup shadows ourselves on Windows. r=dao,Gijs

This prevents long-standing artifacts on HiDPI screens on context menus and so
on.

This is much simpler than tooltips because menupopups and panels already have
the whole set-up ready.

The searchbar.css changes are kind of annoying, but they improve the rendering
a lot (see attached screenshots).

Let me know if you want me to draw shadows on all panels on Windows like we do
on Linux, it's probably doable and cleaner over-all.

Differential Revision: https://phabricator.services.mozilla.com/D145818

2022-06-07T20:22:02.334000: DEBUG : Did not find a branch, checking all integration branches
2022-06-07T20:22:02.334000: INFO : The bisection is done.
2022-06-07T20:22:02.334000: INFO : Stopped

Comment 5

[Mayank Bansal]

Attachment: about:support

Comment 6

[Release mgmt bot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1768278

Comment 7

[Mayank Bansal]

On my system, this repros on 1.5DPI on a 2240x1400 resolution display

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Thanks, I'll try to repro on Windows. If you can show it to me, it'd be helpful to know what the behavior before the patch was? It seems the popup is snapping to the correct area afaict, so not sure why that patch should have an impact.

Comment 9

[Mayank Bansal]

Attachment: Pre-regression Behaviour.mp4

Comment 10

[Mayank Bansal]

This may also have something to do with the bookmark backups in the .JSON format. If I create the bookmark backup in a .HTML format and import it to a new profile, the bug doesnt repro.

Comment 11

[Mayank Bansal]

Attachment: bookmarks.html

Comment 12

[Emilio Cobos Álvarez (:emilio)]

I could repro with 150% scale with a 1600x900 resolution in a near-the-top position. I don't think this is really a regression from my change (or rather, I think this could happen before in different circumstances).

scrollbox.bottom = 590.3332977294922
item.bottom = 590.6666870117188

This code rounds:

https://searchfox.org/mozilla-central/rev/7e34cb7a0094a2f325a0c9db720cec0a2f2aca4f/toolkit/content/widgets/arrowscrollbox.js#592-595

let leftOrTopEdge = ele =>
  Math.round(this._boundsWithoutFlushing(ele)[leftOrTop]);
let rightOrBottomEdge = ele =>
  Math.round(this._boundsWithoutFlushing(ele)[rightOrBottom]);

So the rounding goes in a different direction and we don't think we've reached the bottom even though we have.

Comment 13

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1773052 - Deal better with subpixel differences in arrowscrollbox. r=Gijs,dao

See comment in the bug for the explanation. I could make the epsilon 0.5 and
that should be enough too, but anything less than a single pixel is probably
enough.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/25e8ddbb49fb
Deal better with subpixel differences in arrowscrollbox. r=Gijs

Comment 15

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/25e8ddbb49fb

Comment 16

[Release mgmt bot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:emilio, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 17

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9280265 [details]
Bug 1773052 - Deal better with subpixel differences in arrowscrollbox. r=Gijs,dao

Beta/Release Uplift Approval Request

• User impact if declined: At some resolutions and positions, bottom of some menus might be inaccessible. This is a pre-existing, long-standing bug but it seems recent changes to Windows menus might have made this a bit more frequent.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: On a screen with 150% scale and a 1600x900 resolution, or the setup in comment 7, follow the steps in comment 0.
• List of other uplifts needed: none
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Relatively simple tweak to be more resilient about minor subpixel differences.
• String changes made/needed: none
• Is Android affected?: No

Comment 18

[Mayank Bansal]

This is fixed in latest Nightly. Thanks Emilio!

Comment 19

[Pascal Chevrel:pascalc (PTO until August 22)]

Comment on attachment 9280265 [details]
Bug 1773052 - Deal better with subpixel differences in arrowscrollbox. r=Gijs,dao

Approved for 102 beta 7, thanks.

Comment 20

[Pascal Chevrel:pascalc (PTO until August 22)]

https://hg.mozilla.org/releases/mozilla-beta/rev/bd412ea2768e

Comment 21

[Giorgia Nichita, Release Desktop QA]

Hi, Mayank!
Are you still seeing this issue on the fixed builds, Beta 102.0b7 and the latest Nightly 103.0a1 on your end?

It seems that I'm not able to reproduce the issue with your STR from comment 0. I've tested on Win 10 x64, Ubuntu 21.04 and macOS 10.14, using an affected Nightly build 20220607093440, with different combinations of scales and resolutions, including the ones mentioned in the above comments.

Comment 22

[Mayank Bansal]

(In reply to Giorgia Nichita, Release Desktop QA from comment #21)

Hi, Mayank!
Are you still seeing this issue on the fixed builds, Beta 102.0b7 and the latest Nightly 103.0a1 on your end?

It seems that I'm not able to reproduce the issue with your STR from comment 0. I've tested on Win 10 x64, Ubuntu 21.04 and macOS 10.14, using an affected Nightly build 20220607093440, with different combinations of scales and resolutions, including the ones mentioned in the above comments.

This was fixed in Nightly by Emilio's patch

Comment 23

[Giorgia Nichita, Release Desktop QA]

Thank you for verifying this on Nightly. Just to be sure that everything it's ok on beta as well, could you please also verify it on 102.0b7 so I can close this bug as verified fixed?

Comment 24

[Mayank Bansal]

This doesnt repro for me on the latest Beta available on Firefox homepage.

Comment 25

[Giorgia Nichita, Release Desktop QA]

Thank you!
Closing this bug as verified fixed.