Bug 191659

Summary: Web Inspector: Audit: automatically add to tab bar when the experimental setting is enabled
Product: WebKit Reporter: Devin Rousso <hi>
Component: Web InspectorAssignee: Devin Rousso <hi>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, hi, inspector-bugzilla-changes, joepeck, mattbaker, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
See Also: https://bugs.webkit.org/show_bug.cgi?id=183420
Bug Depends on: 190754    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch none

Devin Rousso
Reported 2018-11-14 14:53:03 PST
Having to enable the experimental setting, reload WebInspector, and then right-click the tab bar to access the new tab is unnecessary. It should automatically be added when enabling the setting.
Attachments
Patch (4.45 KB, patch)
2018-11-14 15:05 PST, Devin Rousso 		no flags
DetailsFormatted DiffDiff
Patch (4.34 KB, patch)
2018-11-25 14:49 PST, Devin Rousso 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Devin Rousso
Comment 1 2018-11-14 15:05:21 PST
Created attachment 354856 [details] Patch
Matt Baker
Comment 2 2018-11-25 13:32:12 PST
Comment on attachment 354856 [details] Patch Does this need to be rebased? it doesn't apply locally for me, and doesn't match trunk: https://trac.webkit.org/browser/webkit/trunk/Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js That aside, I like the idea of automatically adding tabs when enabling the corresponding experimental setting. i wonder about the following case though: - User enables the Layers Tab (for example) - Layers tab is automatically added - User hides Layers tab, leaving the setting enabled - Reload Web Inspector => Layers tab will be added again Ideally we would want to honor the user's decision to hide the tab. Maybe this isn't an issue, since these are experimental features.
Devin Rousso
Comment 3 2018-11-25 14:48:30 PST
(In reply to Matt Baker from comment #2) > That aside, I like the idea of automatically adding tabs when enabling the corresponding experimental setting. i wonder about the following case though: > > - User enables the Layers Tab (for example) > - Layers tab is automatically added > - User hides Layers tab, leaving the setting enabled > - Reload Web Inspector > > => Layers tab will be added again > > Ideally we would want to honor the user's decision to hide the tab. Maybe > this isn't an issue, since these are experimental features. The tab would be hidden, as `WI.AuditTabContentView.isTabAllowed` only returns true if the experimental setting is enabled (the same is true for the layers tab as well).
Devin Rousso
Comment 4 2018-11-25 14:49:42 PST
Created attachment 355606 [details] Patch Rebase
Joseph Pecoraro
Comment 5 2018-11-26 09:12:56 PST
Comment on attachment 355606 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=355606&action=review r=me > Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js:267 > + let newTabs = WI._openTabsSetting.value.slice(); Maybe move this to WI.settings, or public? It’s default value though uses a bunch of other classes.
Devin Rousso
Comment 6 2018-11-26 10:16:11 PST
Comment on attachment 355606 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=355606&action=review >> Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js:267 >> + let newTabs = WI._openTabsSetting.value.slice(); > > Maybe move this to WI.settings, or public? It’s default value though uses a bunch of other classes. Frankly, this is a bit of a "hack", so I'm not as concerned with the idea of public/private, not to mention the fact that if/when these are made non-experimental, the code here will disappear.
WebKit Commit Bot
Comment 7 2018-11-26 10:21:03 PST
Comment on attachment 355606 [details] Patch Clearing flags on attachment: 355606 Committed r238500: <https://trac.webkit.org/changeset/238500>
WebKit Commit Bot
Comment 8 2018-11-26 10:21:04 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 9 2018-11-26 10:22:34 PST
<rdar://problem/46248193>
Note You need to log in before you can comment on or make changes to this bug.