Bug 111061 - Web Inspector: Adding existing key in DOMStorageItemsView leaves it inconsistent state
Summary: Web Inspector: Adding existing key in DOMStorageItemsView leaves it inconsist...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Vivek Galatage
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-28 02:31 PST by Vivek Galatage
Modified: 2013-02-28 08:59 PST (History)
9 users (show)

See Also:


Attachments
Patch (2.31 KB, patch)
2013-02-28 02:34 PST, Vivek Galatage
no flags Details | Formatted Diff | Diff
Patch (2.16 KB, patch)
2013-02-28 03:25 PST, Vivek Galatage
no flags Details | Formatted Diff | Diff
Patch (2.16 KB, patch)
2013-02-28 03:34 PST, Vivek Galatage
no flags Details | Formatted Diff | Diff
Patch (2.15 KB, patch)
2013-02-28 03:39 PST, Vivek Galatage
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Vivek Galatage 2013-02-28 02:31:16 PST
Steps:
1. Create some local storage entries. e.g. { "item1": "value1", "item2": "value2", ... }
2. Navigate to Resources panel and localStorage view
3. Using the last row of the grid (creation node), just enter the value to be one of the existing keys [ "key1", "key2" ... ]
4. Press enter

Expected outcome:
Only one entry per key should be shown

Actual Outcome:
Multiple entries are listed

Patch follows.
Comment 1 Vivek Galatage 2013-02-28 02:34:58 PST
Created attachment 190687 [details]
Patch
Comment 2 Alexander Pavlov (apavlov) 2013-02-28 03:08:17 PST
Comment on attachment 190687 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=190687&action=review

> Source/WebCore/inspector/front-end/DOMStorageItemsView.js:149
> +                if (!keyFound) {

Please use early returns as extensively as possible. It improves the code readability and makes it [a bit more] error-proof.

if (keyFound) {
    childNode.removeSelf();
    continue;
}

keyFound = true;
childNode.data.value = storageData.newValue;
childNode.refresh();
childNode.select();
childNode.reveal();
this.deleteButton.visible = true;
Comment 3 Vivek Galatage 2013-02-28 03:25:47 PST
Created attachment 190691 [details]
Patch
Comment 4 Alexander Pavlov (apavlov) 2013-02-28 03:31:30 PST
Comment on attachment 190691 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=190691&action=review

> Source/WebCore/inspector/front-end/DOMStorageItemsView.js:150
> +                    childNode.parent.removeChild(childNode);

Sorry for missing this. You can replace "childNode.parent" by "rootNode", right?
Comment 5 Vivek Galatage 2013-02-28 03:34:14 PST
Created attachment 190692 [details]
Patch
Comment 6 Vivek Galatage 2013-02-28 03:39:52 PST
Created attachment 190694 [details]
Patch
Comment 7 WebKit Review Bot 2013-02-28 08:58:57 PST
Comment on attachment 190694 [details]
Patch

Clearing flags on attachment: 190694

Committed r144317: <http://trac.webkit.org/changeset/144317>
Comment 8 WebKit Review Bot 2013-02-28 08:59:01 PST
All reviewed patches have been landed.  Closing bug.