Bug 153971 - Web Inspector: WebInspector.Setting should have a "reset" method
Summary: Web Inspector: WebInspector.Setting should have a "reset" method
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Matt Baker
URL:
Keywords: EasyFix, InRadar
Depends on:
Blocks:
 
Reported: 2016-02-07 19:25 PST by Matt Baker
Modified: 2016-02-10 14:57 PST (History)
8 users (show)

See Also:

Attachments
[Patch] Proposed Fix (803 bytes, patch)
2016-02-07 19:49 PST, Matt Baker 		no flags Details | Formatted Diff | Diff
[Patch] Proposed Fix (1.81 KB, patch)
2016-02-07 19:51 PST, Matt Baker 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Matt Baker 2016-02-07 19:25:12 PST 
* SUMMARY
WebInspector.Setting should have a "reset" method. Restoring a setting to it's default value would come in handy for the future Settings tab (https://bugs.webkit.org/show_bug.cgi?id=149284).
Comment 1 Radar WebKit Bug Importer 2016-02-07 19:25:24 PST 
<rdar://problem/24544101>
Comment 2 Radar WebKit Bug Importer 2016-02-07 19:49:28 PST 
<rdar://problem/24544167>
Comment 3 Matt Baker 2016-02-07 19:49:35 PST 
Created attachment 270839 [details]
[Patch] Proposed Fix
Comment 4 Matt Baker 2016-02-07 19:51:18 PST 
Created attachment 270840 [details]
[Patch] Proposed Fix
Comment 5 BJ Burg 2016-02-08 09:36:24 PST 
Comment on attachment 270840 [details]
[Patch] Proposed Fix

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

r=me

> Source/WebInspectorUI/UserInterface/Base/Setting.js:97
> +        this.value = JSON.parse(JSON.stringify(this._defaultValue));

Can we extract this to Utilities.js, something like Object.deepCopyIfSerializable ? We do this idiom in several places, but we never check that _defaultValue is actually serializable, so this can throw an exception. The helper could be more paranoid.
Comment 6 WebKit Commit Bot 2016-02-08 10:23:11 PST 
Comment on attachment 270840 [details]
[Patch] Proposed Fix

Clearing flags on attachment: 270840

Committed r196255: <http://trac.webkit.org/changeset/196255>
Comment 7 WebKit Commit Bot 2016-02-08 10:23:15 PST 
All reviewed patches have been landed.  Closing bug.
Comment 8 Joseph Pecoraro 2016-02-09 15:30:05 PST 
Comment on attachment 270840 [details]
[Patch] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Base/Setting.js:94
> +    reset()

This could, and should, have a test. LayoutTests/inspector/unit-tests/setting.html
Comment 9 Matt Baker 2016-02-10 14:57:18 PST 
(In reply to comment #8)
> Comment on attachment 270840 [details]
> [Patch] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=270840&action=review
> 
> > Source/WebInspectorUI/UserInterface/Base/Setting.js:94
> > +    reset()
> 
> This could, and should, have a test.
> LayoutTests/inspector/unit-tests/setting.html

https://bugs.webkit.org/show_bug.cgi?id=154092