Bug 113198 – Web Inspector: provide minimized mode.

• Status: RESOLVED
• Resolution: WONTFIX
• Component: Web Inspector (Deprecated)

[Dmitry Gozman]

Web Inspector: provide minimized mode.

[Dmitry Gozman]

Created attachment 194854 [details]
Patch

[WebKit Review Bot]

Attachment 194854 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/inspector/InspectorFrontendClient.h', u'Source/WebCore/inspector/InspectorFrontendHost.cpp', u'Source/WebCore/inspector/front-end/DockController.js', u'Source/WebCore/inspector/front-end/Drawer.js', u'Source/WebCore/inspector/front-end/InspectorView.js', u'Source/WebCore/inspector/front-end/ProfilesPanel.js', u'Source/WebCore/inspector/front-end/ScriptsPanel.js', u'Source/WebCore/inspector/front-end/Settings.js', u'Source/WebCore/inspector/front-end/SettingsScreen.js', u'Source/WebCore/inspector/front-end/TimelinePanel.js', u'Source/WebCore/inspector/front-end/Toolbar.js', u'Source/WebCore/inspector/front-end/inspector.css', u'Source/WebCore/inspector/front-end/inspector.html', u'Source/WebCore/inspector/front-end/inspector.js', u'Source/WebKit/chromium/ChangeLog', u'Source/WebKit/chromium/src/InspectorFrontendClientImpl.cpp', u'Source/WebKit/chromium/src/js/DevTools.js']" exit_code: 1
Source/WebCore/inspector/InspectorFrontendClient.h:47:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/inspector/InspectorFrontendClient.h:48:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 2 in 18 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Vsevolod Vlasov]

Comment on attachment 194854 [details]
Patch

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

> Source/WebCore/inspector/front-end/DockController.js:106
> +     * Minmizes/restores devtools.

Please remove comments and leave compiler annotations only.

> Source/WebCore/inspector/front-end/DockController.js:118
> +     * @return {boolean} Whether devtools are in minimized mode.

Ditto

> Source/WebCore/inspector/front-end/DockController.js:125
> +     * Restores from minimized state.

Ditto

> Source/WebCore/inspector/front-end/DockController.js:133
> +     * Minimizes devtools.

Ditto

[Dmitry Gozman]

Comment on attachment 194854 [details]
Patch

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

>> Source/WebCore/inspector/front-end/DockController.js:106
>> +     * Minmizes/restores devtools.
> 
> Please remove comments and leave compiler annotations only.

Done here and below.

[Dmitry Gozman]

Created attachment 195251 [details]
Patch

[WebKit Review Bot]

Attachment 195251 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/inspector/InspectorFrontendClient.h', u'Source/WebCore/inspector/InspectorFrontendHost.cpp', u'Source/WebCore/inspector/front-end/DockController.js', u'Source/WebCore/inspector/front-end/Drawer.js', u'Source/WebCore/inspector/front-end/InspectorView.js', u'Source/WebCore/inspector/front-end/ProfilesPanel.js', u'Source/WebCore/inspector/front-end/ScriptsPanel.js', u'Source/WebCore/inspector/front-end/Settings.js', u'Source/WebCore/inspector/front-end/SettingsScreen.js', u'Source/WebCore/inspector/front-end/TimelinePanel.js', u'Source/WebCore/inspector/front-end/Toolbar.js', u'Source/WebCore/inspector/front-end/inspector.css', u'Source/WebCore/inspector/front-end/inspector.html', u'Source/WebCore/inspector/front-end/inspector.js', u'Source/WebKit/chromium/ChangeLog', u'Source/WebKit/chromium/src/InspectorFrontendClientImpl.cpp', u'Source/WebKit/chromium/src/js/DevTools.js']" exit_code: 1
Source/WebCore/inspector/InspectorFrontendClient.h:47:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/inspector/InspectorFrontendClient.h:48:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 2 in 18 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Pavel Feldman]

Comment on attachment 195251 [details]
Patch

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

> Source/WebCore/inspector/front-end/DockController.js:105
> +    toggleMinimize: function() {

{ should be on the next line.

> Source/WebCore/inspector/front-end/ProfilesPanel.js:381
> +    this.recordButton = new WebInspector.StatusBarButton("", "record-profile-status-bar-item visible-in-minimized");

You should make it a property of StatusBarButton.
this.recordButton.makeVisibleWhenMinimized();

> Source/WebCore/inspector/front-end/SettingsScreen.js:780
> +            WebInspector.dockController.restore();

You should move it into showSettingsScreen.

> Source/WebCore/inspector/front-end/Toolbar.js:392
> +    moveToMinimized: function() {

mimimize

> Source/WebCore/inspector/front-end/Toolbar.js:408
> +    moveFromMinimized: function() {

restore

> Source/WebCore/inspector/front-end/inspector.css:598
> +body.minimized #drawer-view-status-bar > :not(.visible-in-minimized) {

body.mimimized .status-bar-item:not(.visible-when-minimized)

> Source/WebCore/inspector/front-end/inspector.css:616
> +#minimize-button-right {

toolbar-minimize...

[Dmitry Gozman]

Comment on attachment 195251 [details]
Patch

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

>> Source/WebCore/inspector/front-end/DockController.js:105
>> +    toggleMinimize: function() {
> 
> { should be on the next line.

Done.

>> Source/WebCore/inspector/front-end/ProfilesPanel.js:381
>> +    this.recordButton = new WebInspector.StatusBarButton("", "record-profile-status-bar-item visible-in-minimized");
> 
> You should make it a property of StatusBarButton.
> this.recordButton.makeVisibleWhenMinimized();

Done.

>> Source/WebCore/inspector/front-end/SettingsScreen.js:780
>> +            WebInspector.dockController.restore();
> 
> You should move it into showSettingsScreen.

Done.

>> Source/WebCore/inspector/front-end/Toolbar.js:392
>> +    moveToMinimized: function() {
> 
> mimimize

Done.

>> Source/WebCore/inspector/front-end/Toolbar.js:408
>> +    moveFromMinimized: function() {
> 
> restore

Done.

>> Source/WebCore/inspector/front-end/inspector.css:598
>> +body.minimized #drawer-view-status-bar > :not(.visible-in-minimized) {
> 
> body.mimimized .status-bar-item:not(.visible-when-minimized)

There are not only status-bar-items there. For example, breadcrumbs.

>> Source/WebCore/inspector/front-end/inspector.css:616
>> +#minimize-button-right {
> 
> toolbar-minimize...

Done.

[Dmitry Gozman]

Created attachment 195326 [details]
Patch

[WebKit Review Bot]

Attachment 195326 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/inspector/InspectorFrontendClient.h', u'Source/WebCore/inspector/InspectorFrontendHost.cpp', u'Source/WebCore/inspector/front-end/DockController.js', u'Source/WebCore/inspector/front-end/Drawer.js', u'Source/WebCore/inspector/front-end/InspectorView.js', u'Source/WebCore/inspector/front-end/ProfilesPanel.js', u'Source/WebCore/inspector/front-end/ScriptsPanel.js', u'Source/WebCore/inspector/front-end/Settings.js', u'Source/WebCore/inspector/front-end/SettingsScreen.js', u'Source/WebCore/inspector/front-end/StatusBarButton.js', u'Source/WebCore/inspector/front-end/TimelinePanel.js', u'Source/WebCore/inspector/front-end/Toolbar.js', u'Source/WebCore/inspector/front-end/inspector.css', u'Source/WebCore/inspector/front-end/inspector.html', u'Source/WebCore/inspector/front-end/inspector.js', u'Source/WebKit/chromium/ChangeLog', u'Source/WebKit/chromium/src/InspectorFrontendClientImpl.cpp', u'Source/WebKit/chromium/src/js/DevTools.js']" exit_code: 1
Source/WebCore/inspector/InspectorFrontendClient.h:47:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/inspector/InspectorFrontendClient.h:48:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 2 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 195326 [details]
Patch

Attachment 195326 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17351112

[Alexander Pavlov (apavlov)]

Comment on attachment 195326 [details]
Patch

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

> Source/WebCore/inspector/front-end/DockController.js:84
> +        if (this._dockSide && this._dockSide != WebInspector.DockController.State.Minimized)

!==

> Source/WebCore/inspector/front-end/DockController.js:107
> +        if (this.isMinimized()) {

No braces around single-line blocks

> Source/WebCore/inspector/front-end/DockController.js:118
> +    isMinimized: function() {

This is still not fixed.

> Source/WebCore/inspector/front-end/DockController.js:122
> +    restore: function() {

Ditto

> Source/WebCore/inspector/front-end/DockController.js:127
> +    minimize: function() {

Ditto

> Source/WebCore/inspector/front-end/DockController.js:163
> +            if (this._dockSide == WebInspector.DockController.State.Minimized)

===

> Source/WebCore/inspector/front-end/DockController.js:187
> +        if (sides.indexOf(lastState) == -1)

===

> Source/WebCore/inspector/front-end/StatusBarButton.js:212
> +        this.element.addStyleClass("visible-in-minimized");

visible-when-minimized ?

> Source/WebCore/inspector/front-end/Toolbar.js:54
> +        document.getElementById("toolbar-minimize-button-left").style.display = "none";

.addStyleClass("hidden")

> Source/WebCore/inspector/front-end/Toolbar.js:55
> +        document.getElementById("toolbar-minimize-button-right").style.display = "none";

Ditto

> Source/WebCore/inspector/front-end/Toolbar.js:228
> +        return !!WebInspector.dockController && WebInspector.dockController.dockSide() == WebInspector.DockController.State.DockedToRight;

===

> Source/WebCore/inspector/front-end/Toolbar.js:236
> +        return !!WebInspector.dockController && WebInspector.dockController.dockSide() == WebInspector.DockController.State.Minimized;

===

> Source/WebCore/inspector/front-end/Toolbar.js:244
>          return !!WebInspector.dockController && WebInspector.dockController.dockSide() == WebInspector.DockController.State.Undocked;

===

> Source/WebCore/inspector/front-end/Toolbar.js:392
> +    minimize: function() {

Opening brace on a new line

> Source/WebCore/inspector/front-end/Toolbar.js:395
> +        if (controlsRight.parentNode != this.element)

!==

> Source/WebCore/inspector/front-end/Toolbar.js:399
> +        while (this.element.hasChildNodes()) {

No braces around single-line blocks

> Source/WebCore/inspector/front-end/Toolbar.js:408
> +    restore: function() {

Opening brace on a new line

> Source/WebCore/inspector/front-end/Toolbar.js:411
> +        if (controlsRight.parentNode == this.element)

===

> Source/WebCore/inspector/front-end/Toolbar.js:414
> +        while (this._minimizedToolbar.hasChildNodes()) {

No braces...

> Source/WebCore/inspector/front-end/Toolbar.js:418
> +        var firstButton = this.element.querySelectorAll("button")[0];

var firstButton = this.element.querySelector("button");

> Source/WebCore/inspector/front-end/Toolbar.js:419
> +        if (firstButton)

This branching is not necessary. insertBefore(element, null); does what it is expected to.

> Source/WebCore/inspector/front-end/inspector.css:597
> +body.minimized #panel-status-bar > div > :not(.visible-in-minimized),

.visible-when-minimized ?

[Dmitry Gozman]

Comment on attachment 195326 [details]
Patch

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

>> Source/WebCore/inspector/front-end/DockController.js:84
>> +        if (this._dockSide && this._dockSide != WebInspector.DockController.State.Minimized)
> 
> !==

Done.

>> Source/WebCore/inspector/front-end/DockController.js:107
>> +        if (this.isMinimized()) {
> 
> No braces around single-line blocks

Done.

>> Source/WebCore/inspector/front-end/DockController.js:118
>> +    isMinimized: function() {
> 
> This is still not fixed.

Done.

>> Source/WebCore/inspector/front-end/DockController.js:122
>> +    restore: function() {
> 
> Ditto

Done.

>> Source/WebCore/inspector/front-end/DockController.js:127
>> +    minimize: function() {
> 
> Ditto

Done.

>> Source/WebCore/inspector/front-end/DockController.js:163
>> +            if (this._dockSide == WebInspector.DockController.State.Minimized)
> 
> ===

Done.

>> Source/WebCore/inspector/front-end/DockController.js:187
>> +        if (sides.indexOf(lastState) == -1)
> 
> ===

Done.

>> Source/WebCore/inspector/front-end/StatusBarButton.js:212
>> +        this.element.addStyleClass("visible-in-minimized");
> 
> visible-when-minimized ?

Changed. Thanks, missed that.

>> Source/WebCore/inspector/front-end/Toolbar.js:54
>> +        document.getElementById("toolbar-minimize-button-left").style.display = "none";
> 
> .addStyleClass("hidden")

Done.

>> Source/WebCore/inspector/front-end/Toolbar.js:55
>> +        document.getElementById("toolbar-minimize-button-right").style.display = "none";
> 
> Ditto

Done.

>> Source/WebCore/inspector/front-end/Toolbar.js:228
>> +        return !!WebInspector.dockController && WebInspector.dockController.dockSide() == WebInspector.DockController.State.DockedToRight;
> 
> ===

Done.

>> Source/WebCore/inspector/front-end/Toolbar.js:236
>> +        return !!WebInspector.dockController && WebInspector.dockController.dockSide() == WebInspector.DockController.State.Minimized;
> 
> ===

Done.

>> Source/WebCore/inspector/front-end/Toolbar.js:244
>>          return !!WebInspector.dockController && WebInspector.dockController.dockSide() == WebInspector.DockController.State.Undocked;
> 
> ===

Done.

>> Source/WebCore/inspector/front-end/Toolbar.js:392
>> +    minimize: function() {
> 
> Opening brace on a new line

Done.

>> Source/WebCore/inspector/front-end/Toolbar.js:395
>> +        if (controlsRight.parentNode != this.element)
> 
> !==

Done.

>> Source/WebCore/inspector/front-end/Toolbar.js:399
>> +        while (this.element.hasChildNodes()) {
> 
> No braces around single-line blocks

Done.

>> Source/WebCore/inspector/front-end/Toolbar.js:408
>> +    restore: function() {
> 
> Opening brace on a new line

Done.

>> Source/WebCore/inspector/front-end/Toolbar.js:411
>> +        if (controlsRight.parentNode == this.element)
> 
> ===

Done.

>> Source/WebCore/inspector/front-end/Toolbar.js:418
>> +        var firstButton = this.element.querySelectorAll("button")[0];
> 
> var firstButton = this.element.querySelector("button");

Done.

>> Source/WebCore/inspector/front-end/Toolbar.js:419
>> +        if (firstButton)
> 
> This branching is not necessary. insertBefore(element, null); does what it is expected to.

Done.

>> Source/WebCore/inspector/front-end/inspector.css:597
>> +body.minimized #panel-status-bar > div > :not(.visible-in-minimized),
> 
> .visible-when-minimized ?

Fixed.

[Dmitry Gozman]

Created attachment 195567 [details]
Patch

[WebKit Review Bot]

Attachment 195567 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/inspector/InspectorFrontendClient.h', u'Source/WebCore/inspector/InspectorFrontendClientLocal.cpp', u'Source/WebCore/inspector/InspectorFrontendHost.cpp', u'Source/WebCore/inspector/front-end/DockController.js', u'Source/WebCore/inspector/front-end/Drawer.js', u'Source/WebCore/inspector/front-end/InspectElementModeController.js', u'Source/WebCore/inspector/front-end/InspectorView.js', u'Source/WebCore/inspector/front-end/ProfilesPanel.js', u'Source/WebCore/inspector/front-end/ScriptsPanel.js', u'Source/WebCore/inspector/front-end/Settings.js', u'Source/WebCore/inspector/front-end/SettingsScreen.js', u'Source/WebCore/inspector/front-end/StatusBarButton.js', u'Source/WebCore/inspector/front-end/TimelinePanel.js', u'Source/WebCore/inspector/front-end/Toolbar.js', u'Source/WebCore/inspector/front-end/inspector.css', u'Source/WebCore/inspector/front-end/inspector.html', u'Source/WebCore/inspector/front-end/inspector.js', u'Source/WebKit/chromium/ChangeLog', u'Source/WebKit/chromium/src/InspectorFrontendClientImpl.cpp', u'Source/WebKit/chromium/src/js/DevTools.js']" exit_code: 1
Source/WebCore/inspector/InspectorFrontendClient.h:47:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/inspector/InspectorFrontendClient.h:48:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 2 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EFL EWS Bot]

Comment on attachment 195567 [details]
Patch

Attachment 195567 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17296457

[Alexander Pavlov (apavlov)]

Comment on attachment 195567 [details]
Patch

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

Nice, we are almost there.

> Source/WebCore/inspector/front-end/Toolbar.js:53
> +    if (!WebInspector.experimentsSettings.minimizedMode.isEnabled() || !Preferences.canMinimize) {

Would it be a good idea to create the experiment ONLY if Preferences.canMinimize === true?

> Source/WebCore/inspector/front-end/Toolbar.js:394
> +        var controlsLeft = document.getElementById("toolbar-controls-left");

This should be moved next to its usage (I though it was unused at the first glance). If we bail out early, this computation is dropped on the floor.

> Source/WebCore/inspector/front-end/Toolbar.js:396
> +        if (controlsRight.parentNode !== this.element)

parentElement for clarity?

> Source/WebCore/inspector/front-end/Toolbar.js:410
> +        var controlsLeft = document.getElementById("toolbar-controls-left");

This should be moved next to its usage.

> Source/WebCore/inspector/front-end/Toolbar.js:412
> +        if (controlsRight.parentNode === this.element)

parentElement

> Source/WebCore/inspector/front-end/Toolbar.js:422
> +        this._minimizedToolbar.parentNode.removeChild(this._minimizedToolbar);

You can write this._minimizedToolbar.removeSelf() (defined in DOMExtension.js on Element.prototype)

> Source/WebCore/inspector/front-end/inspector.css:630
> +    opacity: 0.7;

It's a bit strange to decrease a button's opacity upon activation (:active implies :hover)

[Dmitry Gozman]

Comment on attachment 195567 [details]
Patch

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

>> Source/WebCore/inspector/front-end/Toolbar.js:53
>> +    if (!WebInspector.experimentsSettings.minimizedMode.isEnabled() || !Preferences.canMinimize) {
> 
> Would it be a good idea to create the experiment ONLY if Preferences.canMinimize === true?

We create experiments before Preferences have been set.

>> Source/WebCore/inspector/front-end/Toolbar.js:394
>> +        var controlsLeft = document.getElementById("toolbar-controls-left");
> 
> This should be moved next to its usage (I though it was unused at the first glance). If we bail out early, this computation is dropped on the floor.

Done.

>> Source/WebCore/inspector/front-end/Toolbar.js:396
>> +        if (controlsRight.parentNode !== this.element)
> 
> parentElement for clarity?

Changed.

>> Source/WebCore/inspector/front-end/Toolbar.js:410
>> +        var controlsLeft = document.getElementById("toolbar-controls-left");
> 
> This should be moved next to its usage.

Done.

>> Source/WebCore/inspector/front-end/Toolbar.js:412
>> +        if (controlsRight.parentNode === this.element)
> 
> parentElement

Changed.

>> Source/WebCore/inspector/front-end/Toolbar.js:422
>> +        this._minimizedToolbar.parentNode.removeChild(this._minimizedToolbar);
> 
> You can write this._minimizedToolbar.removeSelf() (defined in DOMExtension.js on Element.prototype)

Done.

>> Source/WebCore/inspector/front-end/inspector.css:630
>> +    opacity: 0.7;
> 
> It's a bit strange to decrease a button's opacity upon activation (:active implies :hover)

This is to look similar to close button. Anyway, as discussed with pfeldman, we will change icons and behavior of close and minimize buttons to be like toolbar dropdown.

[Build Bot]

Comment on attachment 195567 [details]
Patch

Attachment 195567 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/17311638

[Dmitry Gozman]

Created attachment 195574 [details]
Patch

[WebKit Review Bot]

Attachment 195574 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/inspector/InspectorFrontendClient.h', u'Source/WebCore/inspector/InspectorFrontendClientLocal.cpp', u'Source/WebCore/inspector/InspectorFrontendHost.cpp', u'Source/WebCore/inspector/front-end/DockController.js', u'Source/WebCore/inspector/front-end/Drawer.js', u'Source/WebCore/inspector/front-end/InspectElementModeController.js', u'Source/WebCore/inspector/front-end/InspectorView.js', u'Source/WebCore/inspector/front-end/ProfilesPanel.js', u'Source/WebCore/inspector/front-end/ScriptsPanel.js', u'Source/WebCore/inspector/front-end/Settings.js', u'Source/WebCore/inspector/front-end/SettingsScreen.js', u'Source/WebCore/inspector/front-end/StatusBarButton.js', u'Source/WebCore/inspector/front-end/TimelinePanel.js', u'Source/WebCore/inspector/front-end/Toolbar.js', u'Source/WebCore/inspector/front-end/inspector.css', u'Source/WebCore/inspector/front-end/inspector.html', u'Source/WebCore/inspector/front-end/inspector.js', u'Source/WebKit/chromium/ChangeLog', u'Source/WebKit/chromium/src/InspectorFrontendClientImpl.cpp', u'Source/WebKit/chromium/src/js/DevTools.js', u'Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorFrontendClient.cpp']" exit_code: 1
Source/WebCore/inspector/InspectorFrontendClient.h:47:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/inspector/InspectorFrontendClient.h:48:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 2 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alexander Pavlov (apavlov)]

Comment on attachment 195574 [details]
Patch

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

> Source/WebCore/inspector/front-end/DockController.js:107
> +        if (this.isMinimized()) {

Still not fixed...

> Source/WebCore/inspector/front-end/Toolbar.js:398
> +        this._bottomStatusBarContainer.appendChild(this._minimizedToolbar);

On the second thought, I don't see much sense in keeping this as a field -- we have so many document.getElementById() calls in minimize(), which is the only to use the field.

[Dmitry Gozman]

Comment on attachment 195574 [details]
Patch

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

>> Source/WebCore/inspector/front-end/DockController.js:107
>> +        if (this.isMinimized()) {
> 
> Still not fixed...

Done.

>> Source/WebCore/inspector/front-end/Toolbar.js:398
>> +        this._bottomStatusBarContainer.appendChild(this._minimizedToolbar);
> 
> On the second thought, I don't see much sense in keeping this as a field -- we have so many document.getElementById() calls in minimize(), which is the only to use the field.

I agree. Fixed.

[Dmitry Gozman]

Created attachment 195713 [details]
Patch

[WebKit Review Bot]

Attachment 195713 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/inspector/InspectorFrontendClient.h', u'Source/WebCore/inspector/InspectorFrontendClientLocal.cpp', u'Source/WebCore/inspector/InspectorFrontendHost.cpp', u'Source/WebCore/inspector/front-end/DockController.js', u'Source/WebCore/inspector/front-end/Drawer.js', u'Source/WebCore/inspector/front-end/InspectElementModeController.js', u'Source/WebCore/inspector/front-end/InspectorView.js', u'Source/WebCore/inspector/front-end/ProfilesPanel.js', u'Source/WebCore/inspector/front-end/ScriptsPanel.js', u'Source/WebCore/inspector/front-end/Settings.js', u'Source/WebCore/inspector/front-end/SettingsScreen.js', u'Source/WebCore/inspector/front-end/StatusBarButton.js', u'Source/WebCore/inspector/front-end/TimelinePanel.js', u'Source/WebCore/inspector/front-end/Toolbar.js', u'Source/WebCore/inspector/front-end/inspector.css', u'Source/WebCore/inspector/front-end/inspector.html', u'Source/WebCore/inspector/front-end/inspector.js', u'Source/WebKit/chromium/ChangeLog', u'Source/WebKit/chromium/src/InspectorFrontendClientImpl.cpp', u'Source/WebKit/chromium/src/js/DevTools.js', u'Source/WebKit2/WebProcess/WebCoreSupport/WebInspectorFrontendClient.cpp']" exit_code: 1
Source/WebCore/inspector/InspectorFrontendClient.h:47:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Source/WebCore/inspector/InspectorFrontendClient.h:48:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 2 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 195713 [details]
Patch

Attachment 195713 [details] did not pass win-ews (win):
Output: http://webkit-commit-queue.appspot.com/results/17345954

[Pavel Feldman]

Comment on attachment 195713 [details]
Patch

I applied the patch and there is a list of observations:
- starting / stopping timeline and profiler is awkward: you reach out to minimize, then you start recording, then you stop recording, then you click panel name.
- console drawer does not work when console panel is opened
- we really want command line there
- we need better icons

Another features we could add: upon starting profile / timeline with Shift down, we could minimize inspector. Then, upon stop, we could restore it.