Bug 42816

Summary:

Web Inspector: Modifying settings actually modifies defaultValues

Product:

WebKit

Reporter:

Alexander Pavlov (apavlov) <apavlov>

Component:

Web Inspector (Deprecated)

Assignee:

Alexander Pavlov (apavlov) <apavlov>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bweinstein, joepeck, keishi, pfeldman, pmuellr, rik, timothy, yurys

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Attachments:

Description	Flags
[PATCH] Suggested solution	none
[PATCH] Comment addressed	pfeldman: review-
[PATCH] The correct patch	yurys: review+

Alexander Pavlov (apavlov)

Reported 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).

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
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Alexander Pavlov (apavlov)

Comment 1 2010-07-22 05:41:37 PDT

Created attachment 62288 [details] [PATCH] Suggested solution

Yury Semikhatsky

Comment 2 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?

Alexander Pavlov (apavlov)

Comment 3 2010-07-22 06:34:41 PDT

Created attachment 62291 [details] [PATCH] Comment addressed

Pavel Feldman

Comment 4 2010-07-22 06:44:18 PDT

Comment on attachment 62291 [details] [PATCH] Comment addressed This patch is wrong.

Alexander Pavlov (apavlov)

Comment 5 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

Yury Semikhatsky

Comment 6 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

Alexander Pavlov (apavlov)

Comment 7 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

Note You need to log in before you can comment on or make changes to this bug.