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: PTO - Michael Ratcliffe [:miker] [:mratcliffe]
: Gabriel Luong [:gl][1 biz day review guarantee] (ΦωΦ)
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, PTO - Michael Ratcliffe [:miker] [:mratcliffe]
fayearthur: review+
Details | Diff | Splinter Review

Description User image 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 User image Dave Camp (:dcamp) 2011-12-01 13:17:26 PST
So it does, weird.
Comment 2 User image 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 User image 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 User image PTO - Michael Ratcliffe [:miker] [:mratcliffe] 2012-01-11 05:40:42 PST
Bug triage, filter on PEGASUS.
Comment 5 User image Rob Campbell [:rc] (:robcee) 2012-01-13 10:04:27 PST
Comment on attachment 579906 [details] [diff] [review]

yeah, sure thing.
Comment 6 User image Rob Campbell [:rc] (:robcee) 2012-01-14 09:58:58 PST
Comment 7 User image Tim Taubert [:ttaubert] 2012-01-16 02:30:49 PST
Comment 8 User image Heather Arthur [:harth] 2012-07-12 11:30:15 PDT
Unfortunately, this is happening again.
Comment 9 User image 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 User image PTO - 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 User image PTO - Michael Ratcliffe [:miker] [:mratcliffe] 2013-02-26 04:43:13 PST
Comment 12 User image 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.