Last Comment Bug 706886 - Line-through disappears after editing overridden rule in style inspector
: Line-through disappears after editing overridden rule in style inspector
Product: Firefox
Classification: Client Software
Component: Developer Tools: Inspector (show other bugs)
: unspecified
: All All
: P2 normal (vote)
: Firefox 22
Assigned To: Michael Ratcliffe [:miker] [:mratcliffe]
: Patrick Brosset <:pbro>
Depends on:
  Show dependency treegraph
Reported: 2011-12-01 10:39 PST by Heather Arthur [:harth]
Modified: 2013-07-16 09:34 PDT (History)
6 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

fix (2.50 KB, patch)
2011-12-07 17:16 PST, Dave Camp (:dcamp)
jwalker: review+
rcampbell: feedback+
Details | Diff | Splinter Review
Patch (1.03 KB, patch)
2013-02-21 04:36 PST, Michael Ratcliffe [:miker] [:mratcliffe]
fayearthur: review+
Details | Diff | Splinter Review

Description Heather Arthur [:harth] 2011-12-01 10:39:53 PST
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 Dave Camp (:dcamp) 2011-12-01 13:17:26 PST
So it does, weird.
Comment 2 Dave Camp (:dcamp) 2011-12-07 17:16:07 PST
Created attachment 579906 [details] [diff] [review]

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.
Comment 3 Joe Walker [:jwalker] (needinfo me or ping on irc) 2011-12-09 15:05:24 PST
Comment on attachment 579906 [details] [diff] [review]

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

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

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

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.
Comment 4 Michael Ratcliffe [:miker] [:mratcliffe] 2012-01-11 05:40:42 PST
Bug triage, filter on PEGASUS.
Comment 5 Rob Campbell [:rc] (:robcee) 2012-01-13 10:04:27 PST
Comment on attachment 579906 [details] [diff] [review]

yeah, sure thing.
Comment 6 Rob Campbell [:rc] (:robcee) 2012-01-14 09:58:58 PST
Comment 7 Tim Taubert [:ttaubert] 2012-01-16 02:30:49 PST
Comment 8 Heather Arthur [:harth] 2012-07-12 11:30:15 PDT
Unfortunately, this is happening again.
Comment 9 Dave Camp (:dcamp) 2013-01-22 21:50:18 PST
Not sure when I can get to this, so assigning to default in case someone else wants to pick it up.
Comment 10 Michael Ratcliffe [:miker] [:mratcliffe] 2013-02-21 04:36:11 PST
Created attachment 716483 [details] [diff] [review]

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.
Comment 11 Michael Ratcliffe [:miker] [:mratcliffe] 2013-02-26 04:43:13 PST
Comment 12 Tim Taubert [:ttaubert] 2013-02-26 16:03:11 PST

Note You need to log in before you can comment on or make changes to this bug.