[webkit-reviews] review denied: [Bug 172477] Web Inspector: Add Debug view to Settings tab for debug settings and experimental features : [Attachment 310942] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed May 24 11:22:05 PDT 2017


Joseph Pecoraro <joepeck at webkit.org> has denied Devin Rousso
<drousso at apple.com>'s request for review:
Bug 172477: Web Inspector: Add Debug view to Settings tab for debug settings
and experimental features
https://bugs.webkit.org/show_bug.cgi?id=172477

Attachment 310942: Patch

https://bugs.webkit.org/attachment.cgi?id=310942&action=review




--- Comment #9 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 310942
  --> https://bugs.webkit.org/attachment.cgi?id=310942
Patch

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

>>>> Source/WebInspectorUI/UserInterface/Debug/Bootstrap.js:-40
>>>> -	  let toolTip = "Enable dump inspector messages to console";
>>> 
>>> I still think having this toolbar button is a good idea.
>>> 
>>>	1. You can toggle it from anywhere, which is especially useful if you
only want to debug protocol messages for a short period of time without
switching to the settings tab and back.
>>>	2. It is a global visual indicator that you have debug mode enabled.
>>> 
>>> What do others think?
>> 
>> I have never used it dynamically like that. I always turn this on in
conjunction with dumping console messages to stderr and read it via
Terminal.app.
>> 
>> I do want to keep some sort of visual indicator of whether DebugUI is
enabled or not. I had been using that icon, but we could add something else
like a bug icon that jumps to debug settings or files a pre-filled bug report.
> 
> How about making a debug button in the toolbar that opens the Debug view in
settings when clicked?

I'm going to r-. It doesn't cost us anything to keep this here. So lets just
keep it here. Rest of the patch looks fine to me.


More information about the webkit-reviews mailing list