Cristian Fogel [:cfogel] Reporter
	Description • 5 years ago

[Affected versions]:
- 65.0a1 (2018-12-09), 64.0b14,  63.0.3 
[Affected platforms]:
- win10x64,  Ubuntu 16.04, macOS 10.13 

[Steps to reproduce]:
1. Launch Firefox;
2. Access https://www.wikipedia.org/
3. Open the inspector;
4. Click on the body tag;
5. Delete the overflow-y property and click somewhere else;
6. Click on any tab from the inspector (Layout/Computed/Changes/etc.);
7. Click on the Rules tab;

[Expected result]:
- The body property is still removed;

[Actual result]:
- overflow-x: hidden; is listed under body in the rules section;

[Regression range]:
- last good: 2017-01-13
changeset: 91f5293e9a89056565493ed5073c3842b0ee9fdc
- first bad: 2017-01-15
changeset: 5ce3882eec21be3a70e4afc050959ca2f76bfa76

[Additional notes]:
- 3pane needs to be toggled off;
- attached recording with the issue;
- property is not re-added if the inspector is toggled off->on;

	Patrick Brosset <:pbro>
	Comment 1 • 5 years ago

This can be reproduced with a minimal test case:

1. open data:text/html,<style>body{color:red}</style>test
2. in the rule-view, remove the color:red declaration (by deleting the property or value)
3. switch to another panel
4. go back to the rule-view panel

It always happens when there is only 1 declaration in the rule.
If there are several declarations, then the problem only starts occurring once you've removed all of them.

The culprit is likely this line: https://searchfox.org/mozilla-central/rev/dc3585e58b427c3a8a7c01712fe54ebe118a3ce2/devtools/server/actors/styles.js#1250
where we check if the authoredText for a rule exists simply by checking if it is truthy.
However, if it exists but is empty, then this will be falsy, and we will use the cssText property instead, causing the problem (since it hasn't been updated with the properties being removed).

	Razvan Caliman [:rcaliman] Assignee
	Comment 2 • 5 years ago

(In reply to Patrick Brosset <:pbro> from comment #1)
> This can be reproduced with a minimal test case:
> 
> 1. open data:text/html,<style>body{color:red}</style>test
> 2. in the rule-view, remove the color:red declaration (by deleting the
> property or value)
> 3. switch to another panel
> 4. go back to the rule-view panel

The issue reproduces even without switching to another panel. Just delete the single property, select a different node in the Markup view, then select the original node again. The deleted property is still there.

> The culprit is likely this line: https://searchfox.org/mozilla-central/rev/dc3585e58b427c3a8a7c01712fe54ebe118a3ce2/devtools/server/actors/styles.js#1250

Thanks for the lead! Fixing just that does not solve the problem. I am investigating further.

	Razvan Caliman [:rcaliman] Assignee
	Comment 3 • 5 years ago

> The culprit is likely this line: https://searchfox.org/mozilla-central/rev/dc3585e58b427c3a8a7c01712fe54ebe118a3ce2/devtools/server/actors/styles.js#1250

That was indeed the issue, but there was another similar correction to be made in the front:
https://searchfox.org/mozilla-central/source/devtools/shared/fronts/styles.js#163

	Razvan Caliman [:rcaliman] Assignee
	Comment 4 • 5 years ago

When removing all declarations from a rule via the Rule view, the authoredText value can be an empty string. 
This patch ensures that the fallback cssText is not used in that case since that unintendedly restores the whole declaration block when reparsing the text of the rule.

	Razvan Caliman [:rcaliman] Assignee
	Comment 5 • 5 years ago

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

	Pulsebot
	Comment 10 • 5 years ago

Pushed by rcaliman@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/070cb0982606
Ensure empty string is considered valid CSS authoredText; r=pbro

	Pulsebot
	Comment 13 • 5 years ago

Pushed by rcaliman@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1293aa5ed192
Ensure empty string is considered valid CSS authoredText; r=pbro