Bug 134414 - Web Inspector: Add a setting for clearing the console on page reload
Summary: Web Inspector: Add a setting for clearing the console on page reload
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Enhancement
Assignee: Nobody
Keywords: InRadar
Depends on:
Reported: 2014-06-27 15:04 PDT by Ronald Jett
Modified: 2015-02-20 11:35 PST (History)
5 users (show)

See Also:

patch (5.91 KB, patch)
2014-07-10 12:39 PDT, Ronald Jett
no flags Details | Formatted Diff | Diff
patch (4.08 KB, patch)
2015-02-19 13:44 PST, Ronald Jett
no flags Details | Formatted Diff | Diff
patch (4.01 KB, patch)
2015-02-20 10:22 PST, Ronald Jett
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ronald Jett 2014-06-27 15:04:38 PDT
The Web Inspector should have a setting that will clear the console on reload.
Comment 1 Radar WebKit Bug Importer 2014-06-27 15:04:57 PDT
Comment 2 Ronald Jett 2014-07-10 12:39:54 PDT
Created attachment 234719 [details]
Comment 3 Timothy Hatcher 2014-07-10 13:16:58 PDT
Comment on attachment 234719 [details]

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

> Source/WebInspectorUI/UserInterface/Views/LogContentView.js:113
>      get navigationItems()
>      {
> -        return [this._searchBar, this._scopeBar, this._clearLogNavigationItem, this._toggleSplitNavigationItem];
> +        return [this._searchBar, this._scopeBar, this._clearLogNavigationItem, this._clearLogOnReloadNavigationItem, this._toggleSplitNavigationItem];

I don't think a navigation bar item is the right UI for this at this time. (We would need a new icon that is not confusing with a general Reload.)

Lets add it to the context menu. You can do that in _handleContextMenuEvent by using appendCheckboxItem. You should also do appendSeparator between the existing Clear Log item.

> Source/WebInspectorUI/UserInterface/Views/LogContentView.js:301
> +        if (this._clearLogOnReloadSetting.value) {
> +            this._clearLog();
> +        }

WebKit style is to not use braces for single lines.
Comment 4 Ronald Jett 2015-02-19 13:44:32 PST
Created attachment 246915 [details]
Comment 5 Ronald Jett 2015-02-19 13:47:12 PST
I updated the code, taking in to account your feedback. Sorry it took so long. Let me know what you think.
Comment 6 Timothy Hatcher 2015-02-19 16:07:50 PST
Comment on attachment 246915 [details]

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

> Source/WebInspectorUI/UserInterface/Views/LogContentView.js:75
> +    this._clearLogOnReloadSetting = new WebInspector.Setting("clear-log-on-reload", false);

We might want to default it to true.

> Source/WebInspectorUI/UserInterface/Views/LogContentView.js:343
> +        var clearLogOnReloadUIString = WebInspector.UIString("Clear Log on Reload");
> +
> +        if (this._clearLogOnReloadSetting.value) 
> +            clearLogOnReloadUIString = WebInspector.UIString("Keep Log on Reload");

Nit: Could be one line: this._clearLogOnReloadSetting.value ? WebInspector.UIString("Keep Log on Reload") : WebInspector.UIString("Clear Log on Reload");
Comment 7 Ronald Jett 2015-02-20 10:22:44 PST
Created attachment 246975 [details]
Comment 8 Ronald Jett 2015-02-20 10:23:49 PST
Updated the if to be a ternary. Switching the setting to be true by default.

Is there a WebKit JS style guide somewhere?
Comment 9 Joseph Pecoraro 2015-02-20 10:52:43 PST
> Is there a WebKit JS style guide somewhere?

This gets occasional updates:
Comment 10 WebKit Commit Bot 2015-02-20 11:35:41 PST
Comment on attachment 246975 [details]

Clearing flags on attachment: 246975

Committed r180432: <http://trac.webkit.org/changeset/180432>
Comment 11 WebKit Commit Bot 2015-02-20 11:35:47 PST
All reviewed patches have been landed.  Closing bug.