Changeset 245730 in webkit

Timestamp:

May 23, 2019, 6:34:32 PM (7 years ago)

Author:

Devin Rousso

Message:

Web Inspector: Overlay: rulers/guides should be shown whenever element selection is enabled
​https://bugs.webkit.org/show_bug.cgi?id=198088

Reviewed by Timothy Hatcher.

When trying to "measure" the absolute position (to the viewport) or relative position (to
another element) of a given element, often the easiest way is to enable Element Selection
and Show Rulers at the same time.

This can have the undesired "side-effect" of having the rulers be always present, even when
not highlighting any nodes.

The ideal functionality is to allow the rulers/guides to be shown when element selection is
active and a node is hovered, regardless of whether "Show Rulers" is enabled.

Source/JavaScriptCore:

• inspector/protocol/DOM.json:

Add an optional showRulers parameter to DOM.setInspectModeEnabled that supersedes the
current value of Page.setShowRulers as to whether rulers/guides are shown.

Source/WebCore:

• inspector/InspectorOverlay.h:

(WebCore::InspectorOverlay::setShowRulersDuringElementSelection): Added.

• inspector/InspectorOverlay.cpp:

(WebCore::InspectorOverlay::paint):
(WebCore::InspectorOverlay::shouldShowOverlay):
(WebCore::InspectorOverlay::drawNodeHighlight):
(WebCore::InspectorOverlay::drawQuadHighlight):
(WebCore::InspectorOverlay::drawElementTitle):
If showRulersDuringElementSelection is enabled, draw rulers whenever any highlight bounds
are calculated, but don't update the overlay if it's the only thing enabled (e.g. if there's
no currently hovered node, the overlay will disappear).

• inspector/agents/InspectorDOMAgent.cpp:

(WebCore::InspectorDOMAgent::willDestroyFrontendAndBackend):
(WebCore::InspectorDOMAgent::inspect):
(WebCore::InspectorDOMAgent::setInspectModeEnabled):
(WebCore::InspectorDOMAgent::setSearchingForNode):
Add an optional showRulers parameter to DOM.setInspectModeEnabled that supersedes the
current value of Page.setShowRulers as to whether rulers/guides are shown.

Source/WebInspectorUI:

• UserInterface/Base/Setting.js:
• UserInterface/Views/SettingsTabContentView.js:

(WI.SettingsTabContentView.prototype._createGeneralSettingsView):
Add a setting for controlling whether rulers/guides are shown during element selection.

• UserInterface/Controllers/DOMManager.js:

(WI.DOMManager.prototype.set inspectModeEnabled):
Pass the setting value as an optional parameter when calling DOM.setInspectModeEnabled.

• Localizations/en.lproj/localizedStrings.js:

Location:

trunk/Source

Files:

• JavaScriptCore/ChangeLog (modified) (1 diff)
• JavaScriptCore/inspector/protocol/DOM.json (modified) (1 diff)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/inspector/InspectorOverlay.cpp (modified) (5 diffs)
• WebCore/inspector/InspectorOverlay.h (modified) (2 diffs)
• WebCore/inspector/agents/InspectorDOMAgent.cpp (modified) (5 diffs)
• WebCore/inspector/agents/InspectorDOMAgent.h (modified) (2 diffs)
• WebInspectorUI/ChangeLog (modified) (1 diff)
• WebInspectorUI/Localizations/en.lproj/localizedStrings.js (modified) (2 diffs)
• WebInspectorUI/UserInterface/Base/Setting.js (modified) (1 diff)
• WebInspectorUI/UserInterface/Controllers/DOMManager.js (modified) (1 diff)
• WebInspectorUI/UserInterface/Views/SettingsTabContentView.js (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-05-23  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: Overlay: rulers/guides should be shown whenever element selection is enabled
+        https://bugs.webkit.org/show_bug.cgi?id=198088
+
+        Reviewed by Timothy Hatcher.
+
+        When trying to "measure" the absolute position (to the viewport) or relative position (to
+        another element) of a given element, often the easiest way is to enable Element Selection
+        and Show Rulers at the same time.
+
+        This can have the undesired "side-effect" of having the rulers be always present, even when
+        not highlighting any nodes.
+
+        The ideal functionality is to allow the rulers/guides to be shown when element selection is
+        active and a node is hovered, regardless of whether "Show Rulers" is enabled.
+
+        * inspector/protocol/DOM.json:
+        Add an optional `showRulers` parameter to `DOM.setInspectModeEnabled` that supersedes the
+        current value of `Page.setShowRulers` as to whether rulers/guides are shown.
+
 2019-05-23  Ross Kirsling  <ross.kirsling@sony.com>
 

trunk/Source/JavaScriptCore/inspector/protocol/DOM.json

@@ -401 +401 @@
             "parameters": [
                 { "name": "enabled", "type": "boolean", "description": "True to enable inspection mode, false to disable it." },
-                { "name": "highlightConfig", "$ref": "HighlightConfig", "optional": true, "description": "A descriptor for the highlight appearance of hovered-over nodes. May be omitted if <code>enabled == false</code>." }
+                { "name": "highlightConfig", "$ref": "HighlightConfig", "optional": true, "description": "A descriptor for the highlight appearance of hovered-over nodes. May be omitted if <code>enabled == false</code>." },
+                { "name": "showRulers", "type": "boolean", "optional": true, "description": "Whether the rulers should be shown during element selection. This overrides Page.setShowRulers." }
             ]
         },

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-05-23  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: Overlay: rulers/guides should be shown whenever element selection is enabled
+        https://bugs.webkit.org/show_bug.cgi?id=198088
+
+        Reviewed by Timothy Hatcher.
+
+        When trying to "measure" the absolute position (to the viewport) or relative position (to
+        another element) of a given element, often the easiest way is to enable Element Selection
+        and Show Rulers at the same time.
+
+        This can have the undesired "side-effect" of having the rulers be always present, even when
+        not highlighting any nodes.
+
+        The ideal functionality is to allow the rulers/guides to be shown when element selection is
+        active and a node is hovered, regardless of whether "Show Rulers" is enabled.
+
+        * inspector/InspectorOverlay.h:
+        (WebCore::InspectorOverlay::setShowRulersDuringElementSelection): Added.
+        * inspector/InspectorOverlay.cpp:
+        (WebCore::InspectorOverlay::paint):
+        (WebCore::InspectorOverlay::shouldShowOverlay):
+        (WebCore::InspectorOverlay::drawNodeHighlight):
+        (WebCore::InspectorOverlay::drawQuadHighlight):
+        (WebCore::InspectorOverlay::drawElementTitle):
+        If `showRulersDuringElementSelection` is enabled, draw rulers whenever any highlight bounds
+        are calculated, but don't update the overlay if it's the only thing enabled (e.g. if there's
+        no currently hovered node, the overlay will disappear).
+
+        * inspector/agents/InspectorDOMAgent.cpp:
+        (WebCore::InspectorDOMAgent::willDestroyFrontendAndBackend):
+        (WebCore::InspectorDOMAgent::inspect):
+        (WebCore::InspectorDOMAgent::setInspectModeEnabled):
+        (WebCore::InspectorDOMAgent::setSearchingForNode):
+        Add an optional `showRulers` parameter to `DOM.setInspectModeEnabled` that supersedes the
+        current value of `Page.setShowRulers` as to whether rulers/guides are shown.
+
 2019-05-23  Devin Rousso  <drousso@apple.com>
 

trunk/Source/WebCore/inspector/InspectorOverlay.cpp

@@ -412 +412 @@
         drawPaintRects(context, m_paintRects);
 
-    if (m_showRulers)
+    if (m_showRulers || m_showRulersDuringElementSelection)
         drawRulers(context, bounds);
 }
@@ -493 +493 @@
 bool InspectorOverlay::shouldShowOverlay() const
 {
+    // Don't show the overlay when m_showRulersDuringElementSelection is true, as it's only supposed
+    // to have an effect when element selection is active (e.g. a node is hovered).
     return m_highlightNode || m_highlightNodeList || m_highlightQuad || m_indicating || m_showPaintRects || m_showRulers;
 }
@@ -578 +580 @@
         drawShapeHighlight(context, node, bounds);
 
-    if (m_showRulers)
+    if (m_showRulers || m_showRulersDuringElementSelection)
         drawBounds(context, bounds);
 
@@ -598 +600 @@
         drawOutlinedQuad(context, highlight.quads[0], highlight.contentColor, highlight.contentOutlineColor, bounds);
 
-        if (m_showRulers)
+        if (m_showRulers || m_showRulersDuringElementSelection)
             drawBounds(context, bounds);
     }
@@ -930 +932 @@
     FloatSize contentInset(0, pageView->topContentInset(ScrollView::TopContentInsetType::WebCoreOrPlatformContentInset));
     contentInset.expand(elementDataSpacing, elementDataSpacing);
-    if (m_showRulers)
+    if (m_showRulers || m_showRulersDuringElementSelection)
         contentInset.expand(rulerSize, rulerSize);
 

trunk/Source/WebCore/inspector/InspectorOverlay.h

@@ -116 +116 @@
 
     void setShowRulers(bool);
+    void setShowRulersDuringElementSelection(bool enabled) { m_showRulersDuringElementSelection = enabled; }
 
     Node* highlightedNode() const;
@@ -151 +152 @@
     Timer m_paintRectUpdateTimer;
 
-    bool m_indicating {false};
-    bool m_showPaintRects {false};
-    bool m_showRulers {false};
+    bool m_indicating { false };
+    bool m_showPaintRects { false };
+    bool m_showRulers { false };
+    bool m_showRulersDuringElementSelection { false };
 };
 

trunk/Source/WebCore/inspector/agents/InspectorDOMAgent.cpp

@@ -326 +326 @@
 
     ErrorString unused;
-    setSearchingForNode(unused, false, nullptr);
+    setSearchingForNode(unused, false, nullptr, false);
     hideHighlight(unused);
 
@@ -1095 +1095 @@
     ErrorString unused;
     RefPtr<Node> node = inspectedNode;
-    setSearchingForNode(unused, false, nullptr);
+    setSearchingForNode(unused, false, nullptr, false);
 
     if (node->nodeType() != Node::ELEMENT_NODE && node->nodeType() != Node::DOCUMENT_NODE)
@@ -1148 +1148 @@
 }
 
-void InspectorDOMAgent::setSearchingForNode(ErrorString& errorString, bool enabled, const JSON::Object* highlightInspectorObject)
+void InspectorDOMAgent::setSearchingForNode(ErrorString& errorString, bool enabled, const JSON::Object* highlightInspectorObject, bool showRulers)
 {
     if (m_searchingForNode == enabled)
@@ -1155 +1155 @@
     m_searchingForNode = enabled;
 
-    if (enabled) {
+    m_overlay->setShowRulersDuringElementSelection(m_searchingForNode && showRulers);
+
+    if (m_searchingForNode) {
         m_inspectModeHighlightConfig = highlightConfigFromInspectorObject(errorString, highlightInspectorObject);
         if (!m_inspectModeHighlightConfig)
@@ -1188 +1190 @@
 }
 
-void InspectorDOMAgent::setInspectModeEnabled(ErrorString& errorString, bool enabled, const JSON::Object* highlightConfig)
-{
-    setSearchingForNode(errorString, enabled, highlightConfig ? highlightConfig : nullptr);
+void InspectorDOMAgent::setInspectModeEnabled(ErrorString& errorString, bool enabled, const JSON::Object* highlightConfig, const bool* showRulers)
+{
+    setSearchingForNode(errorString, enabled, highlightConfig ? highlightConfig : nullptr, showRulers && *showRulers);
 }
 

trunk/Source/WebCore/inspector/agents/InspectorDOMAgent.h

@@ -121 +121 @@
     void resolveNode(ErrorString&, int nodeId, const String* objectGroup, RefPtr<Inspector::Protocol::Runtime::RemoteObject>& result) override;
     void getAttributes(ErrorString&, int nodeId, RefPtr<JSON::ArrayOf<String>>& result) override;
-    void setInspectModeEnabled(ErrorString&, bool enabled, const JSON::Object* highlightConfig) override;
+    void setInspectModeEnabled(ErrorString&, bool enabled, const JSON::Object* highlightConfig, const bool* showRulers) override;
     void requestNode(ErrorString&, const String& objectId, int* nodeId) override;
     void pushNodeByPathToFrontend(ErrorString&, const String& path, int* nodeId) override;
@@ -205 +205 @@
 
     void highlightMousedOverNode();
-    void setSearchingForNode(ErrorString&, bool enabled, const JSON::Object* highlightConfig);
+    void setSearchingForNode(ErrorString&, bool enabled, const JSON::Object* highlightConfig, bool showRulers);
     std::unique_ptr<HighlightConfig> highlightConfigFromInspectorObject(ErrorString&, const JSON::Object* highlightInspectorObject);
 

trunk/Source/WebInspectorUI/ChangeLog

@@ +1 @@
+2019-05-23  Devin Rousso  <drousso@apple.com>
+
+        Web Inspector: Overlay: rulers/guides should be shown whenever element selection is enabled
+        https://bugs.webkit.org/show_bug.cgi?id=198088
+
+        Reviewed by Timothy Hatcher.
+
+        When trying to "measure" the absolute position (to the viewport) or relative position (to
+        another element) of a given element, often the easiest way is to enable Element Selection
+        and Show Rulers at the same time.
+
+        This can have the undesired "side-effect" of having the rulers be always present, even when
+        not highlighting any nodes.
+
+        The ideal functionality is to allow the rulers/guides to be shown when element selection is
+        active and a node is hovered, regardless of whether "Show Rulers" is enabled.
+
+        * UserInterface/Base/Setting.js:
+        * UserInterface/Views/SettingsTabContentView.js:
+        (WI.SettingsTabContentView.prototype._createGeneralSettingsView):
+        Add a setting for controlling whether rulers/guides are shown during element selection.
+
+        * UserInterface/Controllers/DOMManager.js:
+        (WI.DOMManager.prototype.set inspectModeEnabled):
+        Pass the setting value as an optional parameter when calling `DOM.setInspectModeEnabled`.
+
+        * Localizations/en.lproj/localizedStrings.js:
+
 2019-05-23  Commit Queue  <commit-queue@webkit.org>
 

trunk/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js

@@ -366 +366 @@
 localizedStrings["Editing audits"] = "Editing audits";
 localizedStrings["Element"] = "Element";
+localizedStrings["Element Selection:"] = "Element Selection:";
 localizedStrings["Element clips compositing descendants"] = "Element clips compositing descendants";
 localizedStrings["Element has CSS blending applied and composited descendants"] = "Element has CSS blending applied and composited descendants";
@@ -958 +959 @@
 localizedStrings["Show page load timing"] = "Show page load timing";
 localizedStrings["Show page resources"] = "Show page resources";
+localizedStrings["Show page rulers and node border lines"] = "Show page rulers and node border lines";
 localizedStrings["Show shadow DOM nodes"] = "Show shadow DOM nodes";
 localizedStrings["Show the details sidebar (%s)"] = "Show the details sidebar (%s)";

trunk/Source/WebInspectorUI/UserInterface/Base/Setting.js

@@ -167 +167 @@
     showPaintRects: new WI.Setting("show-paint-rects", false),
     showRulers: new WI.Setting("show-rulers", false),
+    showRulersDuringElementSelection: new WI.Setting("show-rulers-during-element-selection", true),
     showScopeChainOnPause: new WI.Setting("show-scope-chain-sidebar", true),
     showShadowDOM: new WI.Setting("show-shadow-dom", false),

trunk/Source/WebInspectorUI/UserInterface/Controllers/DOMManager.js

@@ -541 +541 @@
             return;
 
-        DOMAgent.setInspectModeEnabled(enabled, this._buildHighlightConfig(), (error) => {
+        let commandArguments = {
+            enabled,
+            highlightConfig: this._buildHighlightConfig(),
+            showRulers: WI.settings.showRulersDuringElementSelection.value,
+        };
+        DOMAgent.setInspectModeEnabled.invoke(commandArguments, (error) => {
             this._inspectModeEnabled = error ? false : enabled;
             this.dispatchEventToListeners(WI.DOMManager.Event.InspectModeStateChanged);

trunk/Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js

@@ -223 +223 @@
         generalSettingsView.addSeparator();
 
+        generalSettingsView.addSetting(WI.UIString("Element Selection:"), WI.settings.showRulersDuringElementSelection, WI.UIString("Show page rulers and node border lines"));
+
+        generalSettingsView.addSeparator();
+
         const zoomLevels = [0.6, 0.8, 1, 1.2, 1.4, 1.6, 1.8, 2, 2.2, 2.4];
         const zoomValues = zoomLevels.map((level) => [level, Number.percentageString(level, 0)]);