1508748 - Track Changes - Disabling and then deleting a declaration by removing its value is not properly tracked

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P2)

Description

[Paul_B]

Attachment: Multiple records.mp4

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:65.0) Gecko/20100101 Firefox/65.0
Build ID: 20181120100045

[Precondition]

Navigate to about:config and enable the Changes feat. ( devtools.inspector.changes.enabled - true) 

[STR]
1. Launch Firefox, navigate to a random web page (e.g bugzilla.mozilla.org) and open the Inspector.
2. Navigate to the 2nd pane and disable a declaration.
3. Delete the value of the newly disabled property and observe the Changes tab. 
4. Disable another property from the same block and observe the Changes tab.

[AR]
Disabling a property is properly tracked but when the user deletes the value of that disabled property, the Changes tab records the same change again. This issue does not reproduce if the user removes the name of the disabled property.
In some cases, when the user performs the same action within a block, the records multiply. 
  
I have attached a short video for better understanding.

[ER]
Deleting the value of the disabled property is not tracked (as the case is when the same action is performed on the name of the property). There is already a recorded change for disabling/removing the property, and adding another one is redundant. The same rule applies to the 2nd part of the issue, as there is no need for multiple records of the same change.

Comment 1

[Paul_B]

This is not a blocking issue, but I've marked it like this for better visibility and tracking inside the meta bug.

Comment 2

[Paul_B]

I may be wrong, but running Mozregression indicates that this behavior could have been a cause of the changes performed here: https://hg.mozilla.org/integration/autoland/rev/aa12eedecb79

Comment 3

[Paul_B]

Attachment: multiplying records.mp4

On further testing, it would seem that multiplying the records can be triggered even without deleting a new property and can be done by disabling and re-enabling related declarations. I have attached a new short video.

Comment 4

[Razvan Caliman [:rcaliman]]

Attachment: Bug 1508748 - Avoid duplicate tracking of the same removed CSS declaration. r=pbro

When a declaration is disabled, it is tracked as removed in the Changes panel.
When deleting a disabled declaration, we take care not track it as removed again.

Includes stricter checks when matching previously tracked added and removed declarations.

Comment 5

[Razvan Caliman [:rcaliman]]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5ed66766063f968251572607672b8e353cc6a567

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9d1eeceace1b
Avoid duplicate tracking of the same removed CSS declaration. r=pbro

Comment 7

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/9d1eeceace1b