Bug 11920 - Web Inspector should have Firebug-like CSS editing
Summary: Web Inspector should have Firebug-like CSS editing
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 420+
Hardware: All All
: P3 Enhancement
Assignee: Timothy Hatcher
URL:
Keywords:
Depends on: 14377
Blocks:
  Show dependency treegraph
 
Reported: 2006-12-21 11:54 PST by Matt Lilek
Modified: 2007-11-07 10:02 PST (History)
4 users (show)

See Also:


Attachments
Patch (50.04 KB, patch)
2007-11-06 17:43 PST, Timothy Hatcher
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Matt Lilek 2006-12-21 11:54:52 PST
I'm working on implementing live CSS editing from the Web Inspector ala the latest Firebug.  I've got some basic editing working, but it still needs a lot of work so a patch is at least a couple days away.
Comment 1 Adam Roben (:aroben) 2007-06-24 22:51:19 PDT
One part of this is implementing easy disabling of CSS properties, which is bug 14377.
Comment 2 Charles Gaudette 2007-06-24 23:10:01 PDT
This feature request reads as if it is referring to  Firebug's Edit:CSS panel. 

But also note that inside Firebug's Inspect:CSS panel — the same place you can disable properties — it is possible to (temporarily?) insert a new property. You can do this by right-clicking on the CSS rule's name, as in "body {".
Comment 3 Timothy Hatcher 2007-11-06 17:43:59 PST
Created attachment 17072 [details]
Patch
Comment 4 Adam Roben (:aroben) 2007-11-07 01:01:20 PST
Comment on attachment 17072 [details]
Patch

+          and makes the code simplier in the inspector. This function was added for the inspector,

Typo: simplier -> simpler

+          Shrink the toggle zone to 10px to better match the size of the arrow. Add and onattach call

Typo: and -> an

+        if (!x && this.onpopulate && this._expanded) {
+            this.onpopulate(this);
+            this._populated = true;
+        }

Is that first check supposed to be "x" instead of "!x"?

+                // Default editable to true if it was omited.

Typo: omited -> omitted

+        if (shorthand && !used) {
+            // Find out if any of the individual longhand properties of the shorthand
+            // are used, if none are then the shorthand is overloaded too.
+            var longhandProperties = this.styleRule.style.getLonghandProperties(property);
+            for (var j = 0; j < longhandProperties.length; ++j) {
+                var individualProperty = longhandProperties[j];
+                if (individualProperty in this.usedProperties)
+                    return false;
+            }
+
+            return true;
+        }
+
+        return !used;

You could reverse the if and turn this into an early return.

+        if (this.expanded)
+            this.collapse();

Is calling collapse() not just a no-op when this.expanded is false? If it is a no-op then there's no need for the if.

+        console.log(userInput + " (" + userInput.length + ")");
+        console.log(parseElement.getAttribute("style") + " (" + parseElement.getAttribute("style").length + ")");
+        console.log(parseElement.style.length);

Did you mean to leave these in?

+            if (this.getPropertyShorthand(individualProperty) !== shorthandProperty || individualProperty in foundProperties)

Assuming that the "in" check is faster than a call to getPropertyShorthand, you should reverse the order of this ||.

r=me!
Comment 5 Timothy Hatcher 2007-11-07 10:00:29 PST
Landed in r27575. http://trac.webkit.org/projects/webkit/changeset/27575
Comment 6 Timothy Hatcher 2007-11-07 10:02:46 PST
(In reply to comment #4)
> +        if (!x && this.onpopulate && this._expanded) {
> +            this.onpopulate(this);
> +            this._populated = true;
> +        }
> 
> Is that first check supposed to be "x" instead of "!x"?

It should be !x, since we want to re-populate right away if we are currently expanded.

Addressed the other comments.