Bug 1271899 - inspector box-model: add div wrapper to close editors on click;r=gl, a=gchang
authorJulian Descottes <jdescottes@mozilla.com>
Mon, 30 May 2016 13:30:06 +0200
changeset 339913 872545dddeb14f79143049bfce8ff74142721381
parent 339912 b27163b500de7b81f75bcb1116e25b55ba577524
child 339914 de766fceaddc192369a0bfd5cafc1cf17b019341
push id6249
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 13:59:36 +0000
treeherdermozilla-beta@bad9d4f5bf7e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgl, gchang
bugs1271899
milestone49.0a2
Bug 1271899 - inspector box-model: add div wrapper to close editors on click;r=gl, a=gchang The XUL tabpanel can no longer receive userfocus to avoid scrollbars stealing the focus. Editors are using the blur event to destroy themselves, which was no longer working if a user click ended up on the tabpanel element. Add a wrapper inside the layout view panel taking 100% width and height to make sure clicks performed outside of the layout container will trigger a blur event on a focused editor. MozReview-Commit-ID: JmZluQ6LzFl
devtools/client/inspector/inspector.xul
devtools/client/inspector/layout/test/browser.ini
devtools/client/inspector/layout/test/browser_layout_editablemodel.js
devtools/client/inspector/layout/test/browser_layout_editablemodel_bluronclick.js
devtools/client/themes/layout.css
--- a/devtools/client/inspector/inspector.xul
+++ b/devtools/client/inspector/inspector.xul
@@ -270,57 +270,59 @@
                 <html:p class="font-css">&usedAs; "<html:span class="font-css-name"></html:span>"</html:p>
                 <html:pre class="font-css-code"></html:pre>
               </html:div>
             </html:section>
           </html:div>
         </tabpanel>
 
         <tabpanel id="sidebar-panel-layoutview" class="devtools-monospace theme-sidebar inspector-tabpanel">
-          <html:div id="layout-container">
-            <html:p id="layout-header">
-              <html:span id="layout-element-size"></html:span>
-              <html:section id="layout-position-group">
-                <html:button class="devtools-button" id="layout-geometry-editor" title="&geometry.button.tooltip;"></html:button>
-                <html:span id="layout-element-position"></html:span>
-              </html:section>
-            </html:p>
+          <html:div id="layout-wrapper">
+            <html:div id="layout-container">
+              <html:p id="layout-header">
+                <html:span id="layout-element-size"></html:span>
+                <html:section id="layout-position-group">
+                  <html:button class="devtools-button" id="layout-geometry-editor" title="&geometry.button.tooltip;"></html:button>
+                  <html:span id="layout-element-position"></html:span>
+                </html:section>
+              </html:p>
 
-            <html:div id="layout-main">
-              <html:span class="layout-legend" data-box="margin" title="&margin.tooltip;">&margin.tooltip;</html:span>
-              <html:div id="layout-margins" data-box="margin" title="&margin.tooltip;">
-                <html:span class="layout-legend" data-box="border" title="&border.tooltip;">&border.tooltip;</html:span>
-                <html:div id="layout-borders" data-box="border" title="&border.tooltip;">
-                  <html:span class="layout-legend" data-box="padding" title="&padding.tooltip;">&padding.tooltip;</html:span>
-                  <html:div id="layout-padding" data-box="padding" title="&padding.tooltip;">
-                    <html:div id="layout-content" data-box="content" title="&content.tooltip;">
+              <html:div id="layout-main">
+                <html:span class="layout-legend" data-box="margin" title="&margin.tooltip;">&margin.tooltip;</html:span>
+                <html:div id="layout-margins" data-box="margin" title="&margin.tooltip;">
+                  <html:span class="layout-legend" data-box="border" title="&border.tooltip;">&border.tooltip;</html:span>
+                  <html:div id="layout-borders" data-box="border" title="&border.tooltip;">
+                    <html:span class="layout-legend" data-box="padding" title="&padding.tooltip;">&padding.tooltip;</html:span>
+                    <html:div id="layout-padding" data-box="padding" title="&padding.tooltip;">
+                      <html:div id="layout-content" data-box="content" title="&content.tooltip;">
+                      </html:div>
                     </html:div>
                   </html:div>
                 </html:div>
+
+                <html:p class="layout-border layout-top"><html:span data-box="border" class="layout-editable" title="border-top"></html:span></html:p>
+                <html:p class="layout-border layout-right"><html:span data-box="border" class="layout-editable" title="border-right"></html:span></html:p>
+                <html:p class="layout-border layout-bottom"><html:span data-box="border" class="layout-editable" title="border-bottom"></html:span></html:p>
+                <html:p class="layout-border layout-left"><html:span data-box="border" class="layout-editable" title="border-left"></html:span></html:p>
+
+                <html:p class="layout-margin layout-top"><html:span data-box="margin" class="layout-editable" title="margin-top"></html:span></html:p>
+                <html:p class="layout-margin layout-right"><html:span data-box="margin" class="layout-editable" title="margin-right"></html:span></html:p>
+                <html:p class="layout-margin layout-bottom"><html:span data-box="margin" class="layout-editable" title="margin-bottom"></html:span></html:p>
+                <html:p class="layout-margin layout-left"><html:span data-box="margin" class="layout-editable" title="margin-left"></html:span></html:p>
+
+                <html:p class="layout-padding layout-top"><html:span data-box="padding" class="layout-editable" title="padding-top"></html:span></html:p>
+                <html:p class="layout-padding layout-right"><html:span data-box="padding" class="layout-editable" title="padding-right"></html:span></html:p>
+                <html:p class="layout-padding layout-bottom"><html:span data-box="padding" class="layout-editable" title="padding-bottom"></html:span></html:p>
+                <html:p class="layout-padding layout-left"><html:span data-box="padding" class="layout-editable" title="padding-left"></html:span></html:p>
+
+                <html:p class="layout-size"><html:span data-box="content" title="&content.tooltip;"></html:span></html:p>
               </html:div>
 
-              <html:p class="layout-border layout-top"><html:span data-box="border" class="layout-editable" title="border-top"></html:span></html:p>
-              <html:p class="layout-border layout-right"><html:span data-box="border" class="layout-editable" title="border-right"></html:span></html:p>
-              <html:p class="layout-border layout-bottom"><html:span data-box="border" class="layout-editable" title="border-bottom"></html:span></html:p>
-              <html:p class="layout-border layout-left"><html:span data-box="border" class="layout-editable" title="border-left"></html:span></html:p>
-
-              <html:p class="layout-margin layout-top"><html:span data-box="margin" class="layout-editable" title="margin-top"></html:span></html:p>
-              <html:p class="layout-margin layout-right"><html:span data-box="margin" class="layout-editable" title="margin-right"></html:span></html:p>
-              <html:p class="layout-margin layout-bottom"><html:span data-box="margin" class="layout-editable" title="margin-bottom"></html:span></html:p>
-              <html:p class="layout-margin layout-left"><html:span data-box="margin" class="layout-editable" title="margin-left"></html:span></html:p>
-
-              <html:p class="layout-padding layout-top"><html:span data-box="padding" class="layout-editable" title="padding-top"></html:span></html:p>
-              <html:p class="layout-padding layout-right"><html:span data-box="padding" class="layout-editable" title="padding-right"></html:span></html:p>
-              <html:p class="layout-padding layout-bottom"><html:span data-box="padding" class="layout-editable" title="padding-bottom"></html:span></html:p>
-              <html:p class="layout-padding layout-left"><html:span data-box="padding" class="layout-editable" title="padding-left"></html:span></html:p>
-
-              <html:p class="layout-size"><html:span data-box="content" title="&content.tooltip;"></html:span></html:p>
-            </html:div>
-
-            <html:div style="display: none">
-              <html:p id="layout-dummy"></html:p>
+              <html:div style="display: none">
+                <html:p id="layout-dummy"></html:p>
+              </html:div>
             </html:div>
           </html:div>
         </tabpanel>
       </tabpanels>
     </tabbox>
   </box>
 </window>
--- a/devtools/client/inspector/layout/test/browser.ini
+++ b/devtools/client/inspector/layout/test/browser.ini
@@ -10,16 +10,17 @@ support-files =
   !/devtools/client/framework/test/shared-head.js
   !/devtools/client/shared/test/test-actor.js
   !/devtools/client/shared/test/test-actor-registry.js
 
 [browser_layout.js]
 [browser_layout_editablemodel.js]
 # [browser_layout_editablemodel_allproperties.js]
 # Disabled for too many intermittent failures (bug 1009322)
+[browser_layout_editablemodel_bluronclick.js]
 [browser_layout_editablemodel_border.js]
 [browser_layout_editablemodel_stylerules.js]
 [browser_layout_guides.js]
 [browser_layout_rotate-labels-on-sides.js]
 [browser_layout_tooltips.js]
 [browser_layout_update-after-navigation.js]
 [browser_layout_update-after-reload.js]
 # [browser_layout_update-in-iframes.js]
--- a/devtools/client/inspector/layout/test/browser_layout_editablemodel.js
+++ b/devtools/client/inspector/layout/test/browser_layout_editablemodel.js
@@ -8,32 +8,29 @@
 // key bindings
 
 const TEST_URI = "<style>" +
   "div { margin: 10px; padding: 3px }" +
   "#div1 { margin-top: 5px }" +
   "#div2 { border-bottom: 1em solid black; }" +
   "#div3 { padding: 2em; }" +
   "#div4 { margin: 1px; }" +
-  "#div5 { margin: 1px; }" +
   "</style>" +
   "<div id='div1'></div><div id='div2'></div>" +
-  "<div id='div3'></div><div id='div4'></div>" +
-  "<div id='div5'></div>";
+  "<div id='div3'></div><div id='div4'></div>";
 
 add_task(function* () {
   yield addTab("data:text/html," + encodeURIComponent(TEST_URI));
   let {inspector, view, testActor} = yield openLayoutView();
 
   yield testEditingMargins(inspector, view, testActor);
   yield testKeyBindings(inspector, view, testActor);
   yield testEscapeToUndo(inspector, view, testActor);
   yield testDeletingValue(inspector, view, testActor);
   yield testRefocusingOnClick(inspector, view, testActor);
-  yield testBluringOnClick(inspector, view);
 });
 
 function* testEditingMargins(inspector, view, testActor) {
   info("Test that editing margin dynamically updates the document, pressing " +
        "escape cancels the changes");
 
   is((yield getStyle(testActor, "#div1", "margin-top")), "",
      "Should be no margin-top on the element.");
@@ -190,29 +187,8 @@ function* testRefocusingOnClick(inspecto
   is((yield getStyle(testActor, "#div4", "margin-top")), "2px",
      "Should have updated the margin.");
   EventUtils.synthesizeKey("VK_RETURN", {}, view.doc.defaultView);
 
   is((yield getStyle(testActor, "#div4", "margin-top")), "2px",
      "Should be the right margin-top on the element.");
   is(span.textContent, 2, "Should have the right value in the box model.");
 }
-
-function* testBluringOnClick(inspector, view) {
-  info("Test that clicking outside the editor blurs it");
-
-  yield selectNode("#div5", inspector);
-
-  let span = view.doc.querySelector(".layout-margin.layout-top > span");
-  is(span.textContent, 1, "Should have the right value in the box model.");
-
-  EventUtils.synthesizeMouseAtCenter(span, {}, view.doc.defaultView);
-  let editor = view.doc.querySelector(".styleinspector-propertyeditor");
-  ok(editor, "Should have opened the editor.");
-
-  info("Click next to the opened editor input.");
-  let rect = editor.getBoundingClientRect();
-  EventUtils.synthesizeMouse(editor, rect.width + 10, rect.height / 2, {},
-    view.doc.defaultView);
-
-  is(view.doc.querySelector(".styleinspector-propertyeditor"), null,
-    "Inplace editor has been removed.");
-}
new file mode 100644
--- /dev/null
+++ b/devtools/client/inspector/layout/test/browser_layout_editablemodel_bluronclick.js
@@ -0,0 +1,73 @@
+/* vim: set ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Test that inplace editors can be blurred by clicking outside of the editor.
+
+const TEST_URI =
+  `<style>
+    #div1 {
+      margin: 10px;
+      padding: 3px;
+    }
+  </style>
+  <div id="div1"></div>`;
+
+add_task(function* () {
+  // Make sure the toolbox is tall enough to have empty space below the layout-container.
+  yield pushPref("devtools.toolbox.footer.height", 500);
+
+  yield addTab("data:text/html," + encodeURIComponent(TEST_URI));
+  let {inspector, view} = yield openLayoutView();
+
+  yield selectNode("#div1", inspector);
+  yield testClickingOutsideEditor(view);
+  yield testClickingBelowContainer(view);
+});
+
+function* testClickingOutsideEditor(view) {
+  info("Test that clicking outside the editor blurs it");
+  let span = view.doc.querySelector(".layout-margin.layout-top > span");
+  is(span.textContent, 10, "Should have the right value in the box model.");
+
+  EventUtils.synthesizeMouseAtCenter(span, {}, view.doc.defaultView);
+  let editor = view.doc.querySelector(".styleinspector-propertyeditor");
+  ok(editor, "Should have opened the editor.");
+
+  info("Click next to the opened editor input.");
+  let onBlur = once(editor, "blur");
+  let rect = editor.getBoundingClientRect();
+  EventUtils.synthesizeMouse(editor, rect.width + 10, rect.height / 2, {},
+    view.doc.defaultView);
+  yield onBlur;
+
+  is(view.doc.querySelector(".styleinspector-propertyeditor"), null,
+    "Inplace editor has been removed.");
+}
+
+function* testClickingBelowContainer(view) {
+  info("Test that clicking below the box-model container blurs it");
+  let span = view.doc.querySelector(".layout-margin.layout-top > span");
+  is(span.textContent, 10, "Should have the right value in the box model.");
+
+  info("Test that clicking below the layout-container blurs the opened editor");
+  EventUtils.synthesizeMouseAtCenter(span, {}, view.doc.defaultView);
+  let editor = view.doc.querySelector(".styleinspector-propertyeditor");
+  ok(editor, "Should have opened the editor.");
+
+  let onBlur = once(editor, "blur");
+  let container = view.doc.querySelector("#layout-container");
+  // Using getBoxQuads here because getBoundingClientRect (and therefore synthesizeMouse)
+  // use an erroneous height of ~50px for the layout container.
+  let bounds = container.getBoxQuads({relativeTo: view.doc})[0].bounds;
+  EventUtils.synthesizeMouseAtPoint(
+    bounds.left + 10,
+    bounds.top + bounds.height + 10,
+    {}, view.doc.defaultView);
+  yield onBlur;
+
+  is(view.doc.querySelector(".styleinspector-propertyeditor"), null,
+    "Inplace editor has been removed.");
+}
--- a/devtools/client/themes/layout.css
+++ b/devtools/client/themes/layout.css
@@ -2,16 +2,22 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/ */
 
 #sidebar-panel-layoutview {
   display: block;
   overflow: auto;
 }
 
+#layout-wrapper {
+  /* The sidebar-panel is not focusable, this wrapper will catch click events in
+     all the empty area around the layout-container */
+  height: 100%;
+}
+
 #layout-container {
   /* The view will grow bigger as the window gets resized, until 400px */
   max-width: 400px;
   margin: 0px auto;
   padding: 0;
   /* "Contain" the absolutely positioned #layout-main element */
   position: relative;
 }