Web Inspector: REGRESSION(r257835): close and undock buttons are shown in remote...
authordrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 20 Mar 2020 19:22:02 +0000 (19:22 +0000)
committerdrousso@apple.com <drousso@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 20 Mar 2020 19:22:02 +0000 (19:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=209346

Reviewed by Timothy Hatcher.

In the case of remote inspection, the frontend is told that docking is not available before
it is even shown (via `InspectorFrontendAPI.setDockingUnavailable`). Additionally, the
backend (`WebKit::RemoteWebInspectorUI`) never tells the frontend what dock side it actually
is (via `InspectorFrontendAPI.setDockSide`), as there would be no point, given that docking
is unavailable, meaning that the frontend must be undocked.

Before r257835, the docking state held by `WI._dockConfiguration` and `WI.docked`, neither
of which would be set as described above. As a result, in `WI._updateDockNavigationItems`
`WI.docked` would be undefined, which is falsy, thereby causing all docking navigation items
to be hidden. After r257835, these were merged into one `WI.dockConfiguration`, which is
compared against `WI.DockConfiguration.Undocked` instead of just being falsy checked,
meaning it would result in `true` which would not hide all of the docking navigation items.

Change the logic of `WI.updateDockingAvailability` such that if the frontend is told that
docking is unavailable, mark the `WI.dockConfiguration` as `WI.DockConfiguration.Undocked`.
This way, the frontend will always have a valid value for `WI.dockConfiguration`.

Additionally, further leverage `InspectorFrontendHost.supportsDockSide` to only create the
docking navigation items that are actually supported by the host.

* UserInterface/Base/Main.js:
(WI.contentLoaded):
(WI.updateDockingAvailability):
(WI.resizeDockedFrameMouseDown):
(WI.dockedConfigurationSupportsSplitContentBrowser):
(WI._updateDockNavigationItems):
(WI._updateTabBarDividers):

* UserInterface/Views/TabBar.js:
(WI.TabBar.get horizontalPadding):
(WI.TabBar.prototype.resetCachedWidths): Added.
When switching dock configurations, we need to reset the cached width of each tab bar item,
as otherwise, a large width cached when undocked can incorrectly be used when docked.

* UserInterface/Views/TabBarItem.js:
(WI.TabBarItem.get horizontalMargin):
Replace negative checks of `WI.dockConfiguration` with positive ones that can't be fooled by
a falsy value.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@258778 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Base/Main.js
Source/WebInspectorUI/UserInterface/Views/TabBar.js
Source/WebInspectorUI/UserInterface/Views/TabBarItem.js

index 537553c..43a2be7 100644 (file)
@@ -1,3 +1,49 @@
+2020-03-20  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: REGRESSION(r257835): close and undock buttons are shown in remote inspector
+        https://bugs.webkit.org/show_bug.cgi?id=209346
+
+        Reviewed by Timothy Hatcher.
+
+        In the case of remote inspection, the frontend is told that docking is not available before
+        it is even shown (via `InspectorFrontendAPI.setDockingUnavailable`). Additionally, the
+        backend (`WebKit::RemoteWebInspectorUI`) never tells the frontend what dock side it actually
+        is (via `InspectorFrontendAPI.setDockSide`), as there would be no point, given that docking
+        is unavailable, meaning that the frontend must be undocked.
+
+        Before r257835, the docking state held by `WI._dockConfiguration` and `WI.docked`, neither
+        of which would be set as described above. As a result, in `WI._updateDockNavigationItems`
+        `WI.docked` would be undefined, which is falsy, thereby causing all docking navigation items
+        to be hidden. After r257835, these were merged into one `WI.dockConfiguration`, which is
+        compared against `WI.DockConfiguration.Undocked` instead of just being falsy checked,
+        meaning it would result in `true` which would not hide all of the docking navigation items.
+
+        Change the logic of `WI.updateDockingAvailability` such that if the frontend is told that
+        docking is unavailable, mark the `WI.dockConfiguration` as `WI.DockConfiguration.Undocked`.
+        This way, the frontend will always have a valid value for `WI.dockConfiguration`.
+
+        Additionally, further leverage `InspectorFrontendHost.supportsDockSide` to only create the
+        docking navigation items that are actually supported by the host.
+
+        * UserInterface/Base/Main.js:
+        (WI.contentLoaded):
+        (WI.updateDockingAvailability):
+        (WI.resizeDockedFrameMouseDown):
+        (WI.dockedConfigurationSupportsSplitContentBrowser):
+        (WI._updateDockNavigationItems):
+        (WI._updateTabBarDividers):
+
+        * UserInterface/Views/TabBar.js:
+        (WI.TabBar.get horizontalPadding):
+        (WI.TabBar.prototype.resetCachedWidths): Added.
+        When switching dock configurations, we need to reset the cached width of each tab bar item,
+        as otherwise, a large width cached when undocked can incorrectly be used when docked.
+
+        * UserInterface/Views/TabBarItem.js:
+        (WI.TabBarItem.get horizontalMargin):
+        Replace negative checks of `WI.dockConfiguration` with positive ones that can't be fooled by
+        a falsy value.
+
 2020-03-19  Nikita Vasilyev  <nvasilyev@apple.com>
 
         Web Inspector: AXI: disabled buttons shouldn't be focusable
index 4b3c7f6..c8f5ee4 100644 (file)
@@ -366,26 +366,34 @@ WI.contentLoaded = function()
 
     WI._togglePreviousDockConfigurationKeyboardShortcut = new WI.KeyboardShortcut(WI.KeyboardShortcut.Modifier.CommandOrControl | WI.KeyboardShortcut.Modifier.Shift, "D", WI._togglePreviousDockConfiguration);
 
-    WI._closeTabBarButton = new WI.ButtonNavigationItem("dock-close", WI.UIString("Close"), "Images/CloseLarge.svg");
-    WI._closeTabBarButton.addEventListener(WI.ButtonNavigationItem.Event.Clicked, WI.close);
-
     let dockingConfigurationNavigationItems = [];
 
-    if (InspectorFrontendHost.supportsDockSide(WI.DockConfiguration.Right) && InspectorFrontendHost.supportsDockSide(WI.DockConfiguration.Left)) {
+    let supportsDockRight = InspectorFrontendHost.supportsDockSide(WI.DockConfiguration.Right);
+    let supportsDockLeft = InspectorFrontendHost.supportsDockSide(WI.DockConfiguration.Left);
+    let supportsDockBottom = InspectorFrontendHost.supportsDockSide(WI.DockConfiguration.Bottom);
+    let supportsUndocked = InspectorFrontendHost.supportsDockSide(WI.DockConfiguration.Undocked);
+
+    if (supportsDockRight || supportsDockLeft || supportsDockBottom) {
+        WI._closeTabBarButton = new WI.ButtonNavigationItem("dock-close", WI.UIString("Close"), "Images/CloseLarge.svg");
+        WI._closeTabBarButton.addEventListener(WI.ButtonNavigationItem.Event.Clicked, WI.close);
+        dockingConfigurationNavigationItems.push(WI._closeTabBarButton);
+    }
+
+    if ((supportsDockRight || supportsDockLeft) && (supportsDockBottom || supportsUndocked)) {
         WI._dockToSideTabBarButton = new WI.ButtonNavigationItem("dock-right", WI.UIString("Dock to side of window"), WI.resolvedLayoutDirection() === WI.LayoutDirection.RTL ? "Images/DockLeft.svg" : "Images/DockRight.svg", 16, 16);
         WI._dockToSideTabBarButton.element.classList.add(WI.Popover.IgnoreAutoDismissClassName);
         WI._dockToSideTabBarButton.addEventListener(WI.ButtonNavigationItem.Event.Clicked, WI.resolvedLayoutDirection() === WI.LayoutDirection.RTL ? WI._dockLeft : WI._dockRight);
         dockingConfigurationNavigationItems.push(WI._dockToSideTabBarButton);
     }
 
-    if (InspectorFrontendHost.supportsDockSide(WI.DockConfiguration.Bottom)) {
+    if (supportsDockBottom && (supportsDockRight || supportsDockLeft || supportsUndocked)) {
         WI._dockBottomTabBarButton = new WI.ButtonNavigationItem("dock-bottom", WI.UIString("Dock to bottom of window"), "Images/DockBottom.svg", 16, 16);
         WI._dockBottomTabBarButton.element.classList.add(WI.Popover.IgnoreAutoDismissClassName);
         WI._dockBottomTabBarButton.addEventListener(WI.ButtonNavigationItem.Event.Clicked, WI._dockBottom);
         dockingConfigurationNavigationItems.push(WI._dockBottomTabBarButton);
     }
 
-    if (InspectorFrontendHost.supportsDockSide(WI.DockConfiguration.Undocked)) {
+    if (supportsUndocked && (supportsDockRight || supportsDockLeft || supportsDockBottom)) {
         WI._undockTabBarButton = new WI.ButtonNavigationItem("undock", WI.UIString("Detach into separate window"), "Images/Undock.svg", 16, 16);
         WI._undockTabBarButton.element.classList.add(WI.Popover.IgnoreAutoDismissClassName);
         WI._undockTabBarButton.addEventListener(WI.ButtonNavigationItem.Event.Clicked, WI._undock);
@@ -426,7 +434,6 @@ WI.contentLoaded = function()
     }
 
     WI.tabBar.addNavigationItemBefore(new WI.GroupNavigationItem([
-        WI._closeTabBarButton,
         ...dockingConfigurationNavigationItems,
         ...inspectedPageControlNavigationItems,
     ]));
@@ -466,12 +473,16 @@ WI.contentLoaded = function()
     WI.consoleManager.addEventListener(WI.ConsoleManager.Event.Cleared, WI._updateConsoleTabBarButtons, WI);
     WI._updateConsoleTabBarButtons();
 
-    WI._dockedResizerElement = document.getElementById("docked-resizer");
-    WI._dockedResizerElement.classList.add(WI.Popover.IgnoreAutoDismissClassName);
-    WI._dockedResizerElement.addEventListener("mousedown", WI._handleDockedResizerMouseDown);
+    if (supportsDockRight || supportsDockLeft || supportsDockBottom) {
+        WI._dockedResizerElement = document.getElementById("docked-resizer");
+        WI._dockedResizerElement.classList.add(WI.Popover.IgnoreAutoDismissClassName);
+        WI._dockedResizerElement.addEventListener("mousedown", WI._handleDockedResizerMouseDown);
+    }
 
-    let undockedTitleAreaElement = document.getElementById("undocked-title-area");
-    undockedTitleAreaElement.addEventListener("mousedown", WI._handleUndockedTitleAreaMouseDown);
+    if (supportsUndocked) {
+        let undockedTitleAreaElement = document.getElementById("undocked-title-area");
+        undockedTitleAreaElement.addEventListener("mousedown", WI._handleUndockedTitleAreaMouseDown);
+    }
 
     WI._dockingAvailable = false;
 
@@ -839,6 +850,11 @@ WI.updateDockingAvailability = function(available)
 {
     WI._dockingAvailable = available;
 
+    if (!WI._dockingAvailable) {
+        WI.updateDockedState(WI.DockConfiguration.Undocked);
+        return;
+    }
+
     WI._updateDockNavigationItems();
 };
 
@@ -881,13 +897,15 @@ WI.updateDockedState = function(side)
 
     WI._updateDockNavigationItems();
 
+    WI.tabBar.resetCachedWidths();
+
     if (!WI.dockedConfigurationSupportsSplitContentBrowser() && !WI.doesCurrentTabSupportSplitContentBrowser())
         WI.hideSplitConsole();
 };
 
 WI.resizeDockedFrameMouseDown = function(event)
 {
-    console.assert(WI.dockConfiguration !== WI.DockConfiguration.Undocked);
+    console.assert(WI.dockConfiguration && WI.dockConfiguration !== WI.DockConfiguration.Undocked);
 
     if (event.button !== 0 || event.ctrlKey)
         return;
@@ -1102,7 +1120,7 @@ WI.isShowingSplitConsole = function()
 
 WI.dockedConfigurationSupportsSplitContentBrowser = function()
 {
-    return WI.dockConfiguration !== WI.DockConfiguration.Bottom;
+    return !WI.dockConfiguration || WI.dockConfiguration !== WI.DockConfiguration.Bottom;
 };
 
 WI.doesCurrentTabSupportSplitContentBrowser = function()
@@ -1863,10 +1881,11 @@ WI._togglePreviousDockConfiguration = function(event)
 
 WI._updateDockNavigationItems = function()
 {
-    let docked = WI.dockConfiguration !== WI.DockConfiguration.Undocked;
+    let docked = WI.dockedConfigurationSupportsSplitContentBrowser && WI.dockConfiguration !== WI.DockConfiguration.Undocked;
 
     if (WI._dockingAvailable || docked) {
-        WI._closeTabBarButton.hidden = !docked;
+        if (WI._closeTabBarButton)
+            WI._closeTabBarButton.hidden = !docked;
         if (WI._dockToSideTabBarButton)
             WI._dockToSideTabBarButton.hidden = WI.dockConfiguration === WI.DockConfiguration.Right || WI.dockConfiguration === WI.DockConfiguration.Left;
         if (WI._dockBottomTabBarButton)
@@ -1874,7 +1893,8 @@ WI._updateDockNavigationItems = function()
         if (WI._undockTabBarButton)
             WI._undockTabBarButton.hidden = WI.dockConfiguration === WI.DockConfiguration.Undocked;
     } else {
-        WI._closeTabBarButton.hidden = true;
+        if (WI._closeTabBarButton)
+            WI._closeTabBarButton.hidden = true;
         if (WI._dockToSideTabBarButton)
             WI._dockToSideTabBarButton.hidden = true;
         if (WI._dockBottomTabBarButton)
@@ -2336,7 +2356,7 @@ WI._updateInspectModeTabBarButton = function()
 
 WI._updateTabBarDividers = function()
 {
-    let closeHidden = WI._closeTabBarButton.hidden;
+    let closeHidden = WI._closeTabBarButton?.hidden;
     let dockToSideHidden = WI._dockToSideTabBarButton?.hidden;
     let dockBottomHidden = WI._dockBottomTabBarButton?.hidden;
     let undockHidden = WI._undockTabBarButton?.hidden;
@@ -3354,3 +3374,5 @@ WI.DockConfiguration = {
     Bottom: "bottom",
     Undocked: "undocked",
 };
+
+WI.DockConfigurationChangedLayoutReason = Symbol("dock-configuration-changed");
index 40e2f54..2e518df 100644 (file)
@@ -72,7 +72,7 @@ WI.TabBar = class TabBar extends WI.View
 
     static get horizontalPadding()
     {
-        return (WI.dockConfiguration !== WI.DockConfiguration.Undocked) ? 8 : 0; // Keep in sync with `body.docked .tab-bar .tabs`
+        return (WI.dockConfiguration === WI.DockConfiguration.Undocked) ? 0 : 8; // Keep in sync with `body.docked .tab-bar .tabs`
     }
 
     // Public
@@ -425,6 +425,12 @@ WI.TabBar = class TabBar extends WI.View
         return this._tabBarItems.filter((item) => !(item instanceof WI.PinnedTabBarItem)).length;
     }
 
+    resetCachedWidths()
+    {
+        for (let tabBarItem of this._tabBarItems)
+            tabBarItem[WI.TabBar.CachedWidthSymbol] = 0;
+    }
+
     // Protected
 
     layout()
index dccb26f..956766f 100644 (file)
@@ -53,7 +53,7 @@ WI.TabBarItem = class TabBarItem
 
     static get horizontalMargin()
     {
-        return (WI.dockConfiguration !== WI.DockConfiguration.Undocked) ? 4 : 0; // Keep in sync with `body.docked .tab-bar > .tabs > .item`
+        return (WI.dockConfiguration === WI.DockConfiguration.Undocked) ? 0 : 4; // Keep in sync with `body.docked .tab-bar > .tabs > .item`
     }
 
     // Public