Closed Bug 1773052 Opened 2 months ago Closed 2 months ago

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

Categories

(Toolkit :: XUL Widgets, defect)

defect

Tracking

()

VERIFIED FIXED
103 Branch
Tracking Status
firefox-esr91 --- unaffected
firefox101 --- unaffected
firefox102 --- verified
firefox103 --- verified

People

(Reporter: mayankleoboy1, Assigned: emilio)

References

(Regression)

Details

(Keywords: regression)

Attachments

(7 files)

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

Regressed by: 1768278

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

Attached file about:support
Flags: needinfo?(emilio)

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

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

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.

Flags: needinfo?(mayankleoboy1)
Flags: needinfo?(mayankleoboy1)

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.

Attached file bookmarks.html
Has Regression Range: --- → yes

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.

Flags: needinfo?(emilio)
Component: Themes → XUL Widgets

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.

Assignee: nobody → emilio
Status: NEW → ASSIGNED
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/25e8ddbb49fb
Deal better with subpixel differences in arrowscrollbox. r=Gijs
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 103 Branch

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.

Flags: needinfo?(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
Flags: needinfo?(emilio)
Attachment #9280265 - Flags: approval-mozilla-beta?
Flags: qe-verify+

This is fixed in latest Nightly. Thanks Emilio!

QA Whiteboard: [qa-triaged]

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

Approved for 102 beta 7, thanks.

Attachment #9280265 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

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.

Flags: needinfo?(mayankleoboy1)

(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

Flags: needinfo?(mayankleoboy1)

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?

Flags: needinfo?(mayankleoboy1)

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

Flags: needinfo?(mayankleoboy1)

Thank you!
Closing this bug as verified fixed.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.