[webkit-reviews] review granted: [Bug 17773] Inspector UI Refresh : [Attachment 20392] Revised Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Apr 8 12:19:59 PDT 2008


Adam Roben (aroben) <aroben at apple.com> has granted Timothy Hatcher
<timothy at hatcher.name>'s request for review:
Bug 17773: Inspector UI Refresh
http://bugs.webkit.org/show_bug.cgi?id=17773

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

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
+    // The messagesElement is the focusable element so clicking in the
document, anywhere
+    // in the console area will make it focused.

This would be a little clearer phrased like this: The messagesElement is the
focusable element so clicking anywhere in the console area will focus the
prompt.

You should use the license from WebCore/html/PreloadScanner.h in all your new
files.

+WebInspector.ElementsPanel.prototype = {
+    toolbarItemClass: "elements",
+
+    get toolbarItemLabel()
+    {
+	 return WebInspector.UIString("Elements");
+    },

Is there a reason we're using a getter for toolbarItemLabel but not for
toolbarItemClass?

WebInspector.Resource.CompareByDescendingSize can just do: return
CompareBySize(a, b) * -1; You could even move that bit of logic up to
ResourceSidebarTreeElement.

Should View's show() and hide() methods be private?

You could break out the WebInspector.elementDrag* changes into a separate patch
if you really wanted to make me happy.

r=me!


More information about the webkit-reviews mailing list