Closed Bug 1539414 Opened 7 months ago Closed 6 months ago

checkboxes are not redrawn on Win7 classic theme

Categories

(Toolkit :: XUL Widgets, defect, P1)

67 Branch
Unspecified
Windows 7
defect

Tracking

()

VERIFIED FIXED
mozilla68
Tracking Status
firefox-esr60 --- unaffected
firefox66 --- unaffected
firefox67 --- verified
firefox68 --- verified

People

(Reporter: mludha, Assigned: surkov)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

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.

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

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.

Status: UNCONFIRMED → NEW
Component: Untriaged → Security: PSM
Ever confirmed: true
OS: Unspecified → Windows 7
Product: Firefox → Core

Are other checkboxes affected or no?

Flags: needinfo?(mludha)

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

Flags: needinfo?(mludha)

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.

Flags: needinfo?(mludha)

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

Also has the same problem.

Flags: needinfo?(mludha)

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

Component: Security: PSM → Untriaged
Product: Core → Firefox
Summary: checkboxes in Edit CA certificate trust settings are not redrawn on Win7 classic theme → checkboxes are not redrawn on Win7 classic theme
Component: Untriaged → Theme

Bug 1455433 is in the range from comment 2.

Component: Theme → XUL Widgets
Product: Firefox → Toolkit
Regressed by: 1455433
Priority: -- → P1

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

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

Flags: needinfo?(enndeakin)

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.

Assignee: nobody → surkov.alexander
Flags: needinfo?(enndeakin)
Attached image 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)

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.

(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.

Flags: needinfo?(surkov.alexander)

(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=

Flags: needinfo?(surkov.alexander)

(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.

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

Flags: needinfo?(camelia.badau)

(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.

Flags: needinfo?(camelia.badau)

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

Flags: needinfo?(surkov.alexander)
Pushed by asurkov@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f0c2b4af4d6d
checkboxes are not redrawn on Win7 classic theme r=Gijs
Status: NEW → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Flags: qe-verify+

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
Flags: needinfo?(surkov.alexander)
Attachment #9058641 - Flags: approval-mozilla-beta?

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.

Attachment #9058641 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

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.

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