1618243 - Checkboxes in the rule view get out of sync

Status: VERIFIED FIXED

(DevTools :: Inspector: Rules, defect, P2)

Description

[Micah [:mtigley] (she/her)]

STR

1. Open Rules view
2. Uncheck any declaration and edit its value
3. Enable the declaration again to apply.

Expected Results
The declaration should appear to be enabled

Actual Results
The declaration is crossed out.

Comment 1

[Micah [:mtigley] (she/her)]

Attachment: out_of_sync_checkboxes.mp4

Comment 2

[Florens Verschelde :fvsch]

We may have regressed this when we started using <input type="checkbox"> elements (which have their own state) instead of a <div /> styled like a checkbox. When we programmatically change the state after editing a value, we probably don't change the input's checked attribute value.

Comment 3

[Patrick Brosset <:pbro>]

Good hunch Florens, I just ran mozregression and it did confirm that this was a regression caused by bug 1491811.

Comment 4

[Tim Nguyen :ntim]

These places might need to change:

https://searchfox.org/mozilla-central/rev/49ed791eec93335abfe6c2880f84c324e73e47e6/devtools/client/inspector/rules/views/text-property-editor.js#928-934
https://searchfox.org/mozilla-central/rev/49ed791eec93335abfe6c2880f84c324e73e47e6/devtools/client/inspector/rules/views/text-property-editor.js#740-747

to use the JS .checked setter instead of the attribute.

Comment 6

[Tim Nguyen :ntim]

Attachment: Bug 1618243 - Use checked property instead of attribute for rules view checkbox.

Comment 7

[Tim Spurway [:tspurway]]

:gl - did you want this to land in 76?

Comment 8

[Gabriel Luong [:gl] (ΦωΦ)]

(In reply to Tim Spurway [:tspurway] from comment #7)

:gl - did you want this to land in 76?

I have pushed the changes.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/de1d34a37e59
Use checked property instead of attribute for rules view checkbox. r=devtools-inspector-reviewers,gl

Comment 10

[Andrei Ciure[:aciure]]

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

Comment 11

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

The patch landed in nightly and beta is affected.
:ntim, 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 12

[Tim Nguyen :ntim]

Comment on attachment 9133371 [details]
Bug 1618243 - Use checked property instead of attribute for rules view checkbox.

Beta/Release Uplift Approval Request

• User impact if declined: annoying bug for web developers, see comment 0
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See 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): Small straightforward patch
• String changes made/needed:

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9133371 [details]
Bug 1618243 - Use checked property instead of attribute for rules view checkbox.

Fixes a graphical glitch in rules view. Approved for 76.0b3.

Comment 14

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/releases/mozilla-beta/rev/831cbd584e90

Comment 15

[Emil Ghitta, QA [:emilghitta]]

I have managed to reproduce this issue using Firefox 75.0a1 (BuildId:20200226213903) on Windows 10 64bit.

This issue is verified fixed using Firefox 77.0a1 (BuildId:20200409131623) and Firefox 76.0b3 (BuildId:20200409223136) on Windows 10 64bit, macOS 10.14 and Ubuntu 18.04 64bit.