[webkit-reviews] review denied: [Bug 21051] Databases panel should turn into a general Storage panel : [Attachment 26312] This patch is addressing comment #2.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Feb 3 13:18:00 PST 2009


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

Attachment 26312: This patch is addressing comment #2.
https://bugs.webkit.org/attachment.cgi?id=26312&action=review

------- Additional Comments from Timothy Hatcher <timothy at hatcher.name>
This patch is looking great! Sorry it took so long to get a review. Marking as
r- for now to get some things fixed based on the comments below.

> +    page->inspectorController()->didUseDomStorage(storageArea.get(), false,
m_frame);

As Sam mentioned, we prefer the spelling DOM to Dom when in the middle of name,
or dom if it is at the beginning. There are a lot of occurrences of this that
should be changed to DOM.

> +    this.localStorageListTreeElement = new
WebInspector.SidebarSectionTreeElement(WebInspector.UIString("LOCAL-STORAGE"),
{}, true);
> +    this.sessionStorageListTreeElement = new
WebInspector.SidebarSectionTreeElement(WebInspector.UIString("SESSION-STORAGE")
, {}, true);

I think these would read better without the hyphen, so "LOCAL STORAGE" and
"SESSION STORAGE".

> +    content: url(Images/domStorage.png);

What does this icon look like? Who created it? Can it be freely added to WebKit
under the BSD license?

> -.database-view {
> +.database-view, .domstorage-view {

I think it might be better to rename .database-view to something generic like
.storage-view, so all the selectors don't need to be complex multi-selectors.

You also need to add the new files to the WebCore.vcproj.


More information about the webkit-reviews mailing list