1815427 - Checkmark from Profile Manager work offline checkbox appear with pixels with display scale set to 125%

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Timea Zsoldos [:tzsoldos], Desktop QA]

Attachment: checkbox.png

Found in

• Firefox 109.0.1

Affected versions

• Firefox 109.0.1
• Firefox 110.0
• Firefox 111.0a1 (2023-02-07)

Tested platforms

• Affected platforms: Windows 10 x64
• Unaffected platforms: Ubuntu 22 x64, macOS 10.15

Preconditions

• Display scale set to 125% and resolution 1920 x 1080.

Steps to reproduce

1. Launch Firefox with Profile Manager.
2. Select an existing profile and check "Work offline" checkbox.

Expected result

• The checkmark from the "Work offline" checkbox appears correctly.

Actual result

• The checkmark from the "Work offline" checkbox appears with pixels and the checkbox
  is bigger with 1px as the below checkbox, see the attached screenshot (checkbox.png).

Regression range
I will come back with regression range ASAP.

Additional notes

• Note that with display scale set to 100% the issue is not present.

Comment 1

[Timea Zsoldos [:tzsoldos], Desktop QA]

Regression range

Found this manually:

• Last good: ID=20211001093323
• First bad: ID=20211002095048
• Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=60b737e0dbfb3a94e8178d2a77707d546c33b5e3&tochange=37fd65de7f23595e5bacf10e25638ef32819e8d3
• Potentially regressed by: Bug 1733565 - Clean up button active system colors
  or: Bug 1733354 - [win] Profile Manager is no longer readable with Dark OS theme

Comment 2

[David Shin[:dshin]]

So after poking around in mozrunner used for mozregression (Namely, do not add the -profile argument here) I managed to confirm the regression window, though for me the range is 1 day off:

 1:53.18 INFO: Last good revision: 45cb924ad91fe330dadd11046cac31a0f7ace4d0 (2021-09-30)
 1:53.18 INFO: First bad revision: 60b737e0dbfb3a94e8178d2a77707d546c33b5e3 (2021-10-01)
 1:53.18 INFO: Pushlog:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=45cb924ad91fe330dadd11046cac31a0f7ace4d0&tochange=60b737e0dbfb3a94e8178d2a77707d546c33b5e3

And I think this range contains the culprit, which is bug 1733563, enabling the native theming for that window. However, the change itself is valid, and I'm pretty sure that it just surfaced the behaviour.

Not 100% what the root cause is, but this feels like an instance of subpixel snapping causing the box to be a bit larger than intended, causing the checkmark to scale by 1.25 and causing the artifact.

Comment 3

[David Shin[:dshin]]

Attachment: Standalone test case

Attaching standalone test case I managed to produce. Need to follow pref as outlined in the file to enable native rendering of checkboxes.
Should be noted that while the "normal" content rect size is 13px, it is 12.8px at 125% scaling. However, for the XUL equivalent, the minimum size is forced to be 13px.

Comment 4

[David Shin[:dshin]]

So the reason for 12.8px size is here.
GetMinimumWidgetSize calls down to Win32 API call GetThemePartSize. At 100% scaling, it returns 13. At 125% scaling, it returns 16. Ultimately when we make another Win32 API call to draw the widget, the rect we give is 16 pixels as well.
So... it's more correct to have the 12.8px box size, and 13px min-size is not necessarily correct at all scales. Specifically, 13px size would be equivalent to 16.25px, leaving the sizing at the mercy of subpixel snapping.

Comment 5

[David Shin[:dshin]]

I've tried removing the entries, and they don't seem to have any immediate negative impact at various OS scaling/firefox zoom levels.
The specific entry was added in a different file which then got merged into this file in bug 1697315, which seems to inherit the min-size from the non-native .checkbox-check entry, and that goes back to rev 1.

Comment 6

[David Shin[:dshin]]

Attachment: Bug 1815427 - Remove min-size requirement for native-themed XUL checkboxes on Windows. r=mconley

Currently specified min-size does not cover cases when users set OS-wide scalingare .

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

Based on comment #2, this bug contains a bisection range found by mozregression. However, the Regressed by field is still not filled.

:dshin, if possible, could you fill the Regressed by field and investigate this regression?

For more information, please visit auto_nag documentation.

Comment 8

[David Shin[:dshin]]

(As mentioned in bug 1815427 comment 2, the "regressing" bug only surfaced the behaviour, but I think there is value in tracking that)

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

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

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/67bf041f975e
Remove min-size requirement for native-themed XUL checkboxes on Windows. r=desktop-theme-reviewers,dao

Comment 11

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/67bf041f975e

Comment 12

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

Reproducible on a 2023-02-15 Nightly build on Windows 10.
Verified as fixed on Firefox 112.0b7(build ID: 20230326180212) and Nightly 113.0a1(build ID: 20230327211143) on Windows 10.