Line-through disappears after editing overridden rule in style inspector

RESOLVED FIXED in Firefox 22

Status

()

Firefox
Developer Tools: Inspector
P2
normal
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: harth, Assigned: miker)

Tracking

unspecified
Firefox 22
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [ruleview])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
If you edit an overridden rule in the ruleview the line-through / strikeout indicator disappears. The line-through should be re-applied after editing the rule.

Comment 1

6 years ago
So it does, weird.
Priority: -- → P2

Updated

6 years ago
Assignee: nobody → dcamp

Comment 2

6 years ago
Created attachment 579906 [details] [diff] [review]
fix

Fairly straightforward:

* During refresh, TextPropertyEditor hides the strikethrough while editing (to avoid looking strange) by checking if there's an active editor on its spans.
* inplace editor was calling done() callback before tearing itself down.
* done() indirectly causes a refresh (good) which saw a still-active editor (bad).

Attached patch tears down the editor before calling the done() callback.
Attachment #579906 - Flags: review?(jwalker)
Attachment #579906 - Flags: feedback?(rcampbell)
Comment on attachment 579906 [details] [diff] [review]
fix

Review of attachment 579906 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/devtools/styleinspector/CssRuleView.jsm
@@ +1284,1 @@
>      }

This is a blatant 'to prove i read it' comment:

this._clear();
if (typeof this.done == "function") {
  let value = this.cancelled ? this.initial : this.input.value.trim();
  this.done(value, !this.cancelled);
}

It's 0.0002ms faster in the case when this.done isn't set, and it doesn't crash if this.done is set incorrectly, and it read easier to me, and I really don't care if you ignore me.
Attachment #579906 - Flags: review?(jwalker) → review+
Bug triage, filter on PEGASUS.
Whiteboard: [ruleview]

Updated

6 years ago
Whiteboard: [ruleview] → [ruleview][land-in-fx-team]
Comment on attachment 579906 [details] [diff] [review]
fix

yeah, sure thing.
Attachment #579906 - Flags: feedback?(rcampbell) → feedback+
https://hg.mozilla.org/integration/fx-team/rev/b8281fe1b926
Status: NEW → ASSIGNED
Whiteboard: [ruleview][land-in-fx-team] → [ruleview][fixed-in-fx-team]
Whiteboard: [ruleview][fixed-in-fx-team] → [ruleview]
https://hg.mozilla.org/mozilla-central/rev/b8281fe1b926
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Reporter)

Comment 8

5 years ago
Unfortunately, this is happening again.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 9

4 years ago
Not sure when I can get to this, so assigning to default in case someone else wants to pick it up.
Assignee: dcamp → nobody

Updated

4 years ago
Blocks: 831711
Assignee: nobody → mratcliffe
Created attachment 716483 [details] [diff] [review]
Patch

Simple fix. Use linethrough if we should when the value is overridden.

We were displaying as overridden when the name was changed, but not the value.
Attachment #579906 - Attachment is obsolete: true
Attachment #716483 - Flags: review?(fayearthur)
Whiteboard: [ruleview] → [ruleview][has-patch]
(Reporter)

Updated

4 years ago
Attachment #716483 - Flags: review?(fayearthur) → review+
Whiteboard: [ruleview][has-patch] → [ruleview][land-in-fx-team]
https://hg.mozilla.org/integration/fx-team/rev/a5c189768303
Whiteboard: [ruleview][land-in-fx-team] → [ruleview][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/a5c189768303
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago4 years ago
Resolution: --- → FIXED
Whiteboard: [ruleview][fixed-in-fx-team] → [ruleview]
Target Milestone: --- → Firefox 22
No longer blocks: 831711
You need to log in before you can comment on or make changes to this bug.