1539414 - checkboxes are not redrawn on Win7 classic theme

Status: VERIFIED FIXED

(Toolkit :: UI Widgets, defect, P1)

Description

[Marek Ludha]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:60.0) Gecko/20100101 Firefox/60.0

Steps to reproduce:

1. In Windows 7 switch System properties -> Performance -> Settings -> Visual Effects to "Adjust for best performance"
2. Open Tools -> Options -> View Certificates -> Edit trust
3. Click any of the checkboxes

Actual results:

There is no apparent effect. Saving the dialog and reopening shows that the changes took effect, but were not displayed. Changing Visual Effects to "Adjust for best appearance" makes the changes visible, also all further changes are displayed immediately.

Expected results:

Changes should be displayed immediately with win 7 classic theme.

Comment 1

[Marek Ludha]

Actual User Agent: Mozilla/5.0 (Windows NT 6.1; rv:67.0) Gecko/20100101 Firefox/67.0

Comment 2

[Paul_B]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:67.0) Gecko/20100101 Firefox/67.0
Build ID: 20190328152334

Hello reporter,

Thank you for taking the time to add this report. I was able to reproduce the behavior you mentioned using the latest DevEdition Build and the latest Nightly build. The same issue did not reproduce on the latest Firefox Release build.

After setting the Visual Effects to "Adjust for best performance", the selection or deselection of the checkboxes were no longer visible, even though saving and then opening them again revealed the fact that the action did take effect and that the checkboxes were selected or deselected on the previous action but without a visual indicator.

Running mozregression tool indicated that the change that might have caused this can be found in the pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=98d4803bb2de7fcdb0096ddeedaefd29d0fd5fd4&tochange=3b08a133c893b960f15e014a8f36c954d31fda34
Unfortunately, I could not pinpoint the exact fix that caused it but from what I can tell it is somewhere between 9d39099e and c1075c1f.

I am going to add this to Core: Security PSM component for an advised input. If this is not the proper component, please feel free to change it to a more appropriate one.

Comment 3

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Are other checkboxes affected or no?

Comment 4

[Marek Ludha]

During the (rather short) time I've spent with FF67 I've only noticed the two on this dialog.

Comment 5

[Dana Keeler (she/her) (use needinfo) [:keeler]]

To clarify, do you mean to say that you've only noticed that these two checkboxes are affected by this bug, or that you've only noticed the existence of these two checkboxes? If the latter, try bookmarking a page - a dialog should pop up with a checkbox.

Comment 6

[Marek Ludha]

If the latter, try bookmarking a page - a dialog should pop up with a checkbox.

Also has the same problem.

Comment 7

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Thanks - that means this isn't specific to PSM.

Comment 8

[Dão Gottwald [:dao]]

Bug 1455433 is in the range from comment 2.

Comment 9

[alexander :surkov (:asurkov)]

Does it sound like styling problem? I don't have win7 machine on hands. Is anyone with win machine around to compare styles between builds?

also see bug 1541045, these two might be correlated

Comment 10

[Pascal Chevrel:pascalc]

Neil, could we get somebody assigned to this regression in 67? Thanks

Comment 11

[Neil Deakin]

As this is a likely a regression from 1455433, Alex should probably look at it. He suggests above that he needs someone, (a QA member?) to help compare builds.

Comment 12

[Camelia Badau [:cbadau], Desktop Test Engineering]

Attachment: issue.gif

I've tested on Windows 7 x64 with Windows Classic theme and "Adjust for best performance" set in Visual Effects.

"As this is a likely a regression from 1455433", I've tested on:

• Nightly 67.0a1, buildID: 20190301052057 (the build without the fix) -> the issue doesn't reproduce here, the changes on checkboxes are displayed immediately
• Nightly 67.0a1, buildID: 20190301201651 (the build containing the fix) -> the issue is present here (I follow the steps from Description)

I've notice the issue on following checkboxes:

• Options -> View Certificates -> Edit trust
• Bookmarking a page (CTRL+D) -> a dialog pop up with a checkbox
• "Default Browser" dialog -> when Firefox is opened, "Default Browser" dialog pop up with a checkbox
• "Exit and close tabs?" dialog -> when Firefox (with several tabs opened) is closed, the Exit dialog pop up with a checkbox
• "Remember this decision" checkbox from Allow Location doorhanger -> Go to "https://www.google.com/maps" and click to show your location - the doorhanger with the checkbox is displayed
• "Show password" checkbox from Password Manager dialog -> Go to "https://twitter.com/" and login - the Password Manager dialog with the checkbox is displayed
• "Remember this decision" checkbox from Allow Flash Player dialog -> Go to "http://www.sjgames.com/revolution/demo.html" and click Run Adobe Flash - the Allow Flash dialog with the checkbox is displayed

It seems that this issue is indeed a regression from bug 1455433.
Please see attachment "issue.gif" - on the left is build without the fix/unaffected build (20190301052057) and on the right is build containing the fix/affected build (20190301201651)

Comment 13

[alexander :surkov (:asurkov)]

Camelia, would you be able to make some checks in devtools please? I suspect that this bug is similar to bug 1541045, which was caused by missing attribute on a checkbox child element, what caused the styling problem. In this case I'm curious if there's a difference in styles of .checkbox-check image element, a child of checkbox.

Comment 14

[Brian Grinstead [:bgrins]]

(In reply to alexander :surkov (:asurkov) from comment #13)

Camelia, would you be able to make some checks in devtools please? I suspect that this bug is similar to bug 1541045, which was caused by missing attribute on a checkbox child element, what caused the styling problem. In this case I'm curious if there's a difference in styles of .checkbox-check image element, a child of checkbox.

Shouldn't we also be inheriting checked at https://hg.mozilla.org/mozilla-central/rev/70ed6fa7f921#l1.12? We used to have [inherits] on that attr in the XBL implementation: https://searchfox.org/mozilla-release/rev/5bc9a3f39fae58ad2f2bdc5abfca04c8e21756e2/toolkit/content/widgets/checkbox.xml#15.

Comment 15

[alexander :surkov (:asurkov)]

(In reply to Brian Grinstead [:bgrins] from comment #14)

(In reply to alexander :surkov (:asurkov) from comment #13)

Camelia, would you be able to make some checks in devtools please? I suspect that this bug is similar to bug 1541045, which was caused by missing attribute on a checkbox child element, what caused the styling problem. In this case I'm curious if there's a difference in styles of .checkbox-check image element, a child of checkbox.

Shouldn't we also be inheriting checked at https://hg.mozilla.org/mozilla-central/rev/70ed6fa7f921#l1.12? We used to have [inherits] on that attr in the XBL implementation: https://searchfox.org/mozilla-release/rev/5bc9a3f39fae58ad2f2bdc5abfca04c8e21756e2/toolkit/content/widgets/checkbox.xml#15.

I thought about that, but I don't see anything in styling [1] that could interfere with that. I can start a try build to check this idea, perhaps it'd be easier than playing with devtools.

[1] https://searchfox.org/mozilla-central/search?q=.checkbox-check&case=true&path=

Comment 16

[Brian Grinstead [:bgrins]]

(In reply to alexander :surkov (:asurkov) from comment #15)

I thought about that, but I don't see anything in styling [1] that could interfere with that. I can start a try build to check this idea, perhaps it'd be easier than playing with devtools.

[1] https://searchfox.org/mozilla-central/search?q=.checkbox-check&case=true&path=

Yeah, it would probably be easiest to do that and send it over to Camelia.

Comment 17

[alexander :surkov (:asurkov)]

Here's a try build that puts back checked attribute inheritance [1]. Camelia, could you give a try and see if it helps?

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=79ecdbcfa7187dd97a9ee123df78f287a590eb35

Comment 18

[Camelia Badau [:cbadau], Desktop Test Engineering]

(In reply to alexander :surkov (:asurkov) from comment #17)

Here's a try build that puts back checked attribute inheritance [1].
Camelia, could you give a try and see if it helps?

[1]
https://treeherder.mozilla.org/#/
jobs?repo=try&revision=79ecdbcfa7187dd97a9ee123df78f287a590eb35

I've tested on Windows 7 x64 with Windows Classic theme and "Adjust for best performance" set in Visual Effects using this try build (https://treeherder.mozilla.org/#/jobs?repo=try&revision=79ecdbcfa7187dd97a9ee123df78f287a590eb35&selectedJob=239964862) and the checkboxes mentioned in comment 12 correctly work now, the changes on these checkboxes are displayed immediately.

Comment 19

[alexander :surkov (:asurkov)]

Attachment: Bug 1539414 - checkboxes are not redrawn on Win7 classic theme

Comment 20

[Pascal Chevrel:pascalc]

Alexander, it seems your patch was r+ 3 days ago, are you going to land it soon so as that we can evaluate an uplift? Thanks

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f0c2b4af4d6d
checkboxes are not redrawn on Win7 classic theme r=Gijs

Comment 22

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/f0c2b4af4d6d

Comment 23

[alexander :surkov (:asurkov)]

Comment on attachment 9058641 [details]
Bug 1539414 - checkboxes are not redrawn on Win7 classic theme

Beta/Release Uplift Approval Request

• User impact if declined: Firefox UI visual defects on win7 (checked checkboxes don't look checked)
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): it is a rollback of part of bug 1455433 which regressed checkboxes
• String changes made/needed: no

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9058641 [details]
Bug 1539414 - checkboxes are not redrawn on Win7 classic theme

Fixes a regression for Win7 users with the classic theme enabled. Approved for 67.0b13.

Comment 25

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/releases/mozilla-beta/rev/9b8ad8b840ed

Comment 26

[Camelia Badau [:cbadau], Desktop Test Engineering]

I've tested on Windows 7 x64 with Windows Classic theme and "Adjust for best performance" set in Visual Effects using latest Nightly 68.0a1 (2019-04-29) and Firefox 67 Beta 15 (buildID: 20190429125729) and all the checkboxes mentioned in comment 12 correctly work now, the changes on these checkboxes are displayed immediately.