[webkit-reviews] review granted: [Bug 11920] Web Inspector should have Firebug-like CSS editing : [Attachment 17072] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Nov 7 01:01:20 PST 2007


Adam Roben <aroben at apple.com> has granted Timothy Hatcher
<timothy at hatcher.name>'s request for review:
Bug 11920: Web Inspector should have Firebug-like CSS editing
http://bugs.webkit.org/show_bug.cgi?id=11920

Attachment 17072: Patch
http://bugs.webkit.org/attachment.cgi?id=17072&action=edit

------- Additional Comments from Adam Roben <aroben at apple.com>
+	   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!


More information about the webkit-reviews mailing list