[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