Bug 42816 - Web Inspector: Modifying settings actually modifies defaultValues
Summary: Web Inspector: Modifying settings actually modifies defaultValues
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: Alexander Pavlov (apavlov)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-07-22 04:23 PDT by Alexander Pavlov (apavlov)
Modified: 2010-07-22 07:26 PDT (History)
8 users (show)

See Also:


Attachments
[PATCH] Suggested solution (2.49 KB, patch)
2010-07-22 05:41 PDT, Alexander Pavlov (apavlov)
no flags Details | Formatted Diff | Diff
[PATCH] Comment addressed (89.19 KB, patch)
2010-07-22 06:34 PDT, Alexander Pavlov (apavlov)
pfeldman: review-
Details | Formatted Diff | Diff
[PATCH] The correct patch (2.65 KB, patch)
2010-07-22 06:55 PDT, Alexander Pavlov (apavlov)
yurys: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Pavlov (apavlov) 2010-07-22 04:23:12 PDT
http://trac.webkit.org/changeset/62647 introduced the _defaultValues array into WebInspector.Settings. If a frontend is loaded and no value has been stored for a certain property (which is the case for all sessionSettings), then the first retrieved property value comes from _defaultValues, and if it is an object, it can be modified by the user. On a subsequent frontend reload the _defaultValues will store a modified property value rather than the default one.
Rolling back of the Settings.js change from the said changeset has been agreed upon with yurys@chromium.org (the changeset author).
Comment 1 Alexander Pavlov (apavlov) 2010-07-22 05:41:37 PDT
Created attachment 62288 [details]
[PATCH] Suggested solution
Comment 2 Yury Semikhatsky 2010-07-22 05:51:27 PDT
Comment on attachment 62288 [details]
[PATCH] Suggested solution

WebCore/inspector/front-end/Settings.js:93
 +          for (var propertyName in loadedStore)
what if loadedStore is undefined?
Comment 3 Alexander Pavlov (apavlov) 2010-07-22 06:34:41 PDT
Created attachment 62291 [details]
[PATCH] Comment addressed
Comment 4 Pavel Feldman 2010-07-22 06:44:18 PDT
Comment on attachment 62291 [details]
[PATCH] Comment addressed

This patch is wrong.
Comment 5 Alexander Pavlov (apavlov) 2010-07-22 06:55:39 PDT
Created attachment 62292 [details]
[PATCH] The correct patch

Sorry, forgot to rebase the branch before creating the previous patch
Comment 6 Yury Semikhatsky 2010-07-22 07:03:40 PDT
Comment on attachment 62292 [details]
[PATCH] The correct patch

WebCore/inspector/front-end/Settings.js:81
 +          // FIXME: restore default values
Please file a bug and put its number next to FIXME
Comment 7 Alexander Pavlov (apavlov) 2010-07-22 07:26:14 PDT
Landed with bug 42820 mentioned.

Committing to http://svn.webkit.org/repository/webkit/trunk ...
        M       WebCore/ChangeLog
        M       WebCore/inspector/front-end/Settings.js
Committed r63889