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 PTO -> Fri, 24 Jun 2016 [:miker] [:mratcliffe]
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 | Review
Patch (1.03 KB, patch)
2013-02-21 04:36 PST, Michael Ratcliffe PTO -> Fri, 24 Jun 2016 [:miker] [:mratcliffe]
fayearthur: review+
Details | Diff | 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 PTO -> Fri, 24 Jun 2016 [: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 PTO -> Fri, 24 Jun 2016 [: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 PTO -> Fri, 24 Jun 2016 [: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.