[webkit-reviews] review denied: [Bug 23866] Storage panel should be editable : [Attachment 27855] Make DOMStorage view editable.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Feb 25 17:57:32 PST 2009


Timothy Hatcher <timothy at hatcher.name> has denied Yael
<yael.aharon at nokia.com>'s request for review:
Bug 23866: Storage panel should be editable
https://bugs.webkit.org/show_bug.cgi?id=23866

Attachment 27855: Make DOMStorage view editable.
https://bugs.webkit.org/attachment.cgi?id=27855&action=review

------- Additional Comments from Timothy Hatcher <timothy at hatcher.name>

> +    get statusBarItems()
> +    {
> +	   return [this.deleteButton];
> +    },

The delete button should be on the view side of the status bar, like we do in
the ProfilesPanel/ProfilesView. A delete button here would imply deleting a
local/session storage object not a row. r- for this

> +	       this.visibleView._dataGrid._deleteRow();

Functions that are "public" like _deleteRow() should not have an underscore.
Also deleteSelectedRow() would be best, since the row index isn't passed.

> +	   var element = event.target.enclosingNodeOrSelfWithNodeName("td");

Why do you need to look for a element? Can't you get everything from
dataGridNodeFromEvent?

> +	       }
> +	       else {

This should be: } else { on the same line.


> +	   if (node && domStorage) {
> +	       domStorage.removeItem(node.data[0]);
> +	   }

No need for braces here.


More information about the webkit-reviews mailing list