Bug 1484110 - part 3: HTMLEditor::RefereshEditingUI() should refresh UIs when one of them is changed to enabled or disabled r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 17 Aug 2018 19:03:02 +0900
changeset 432536 4f0a12bcb4017a46280b133aa2be83ca49b6d14a
parent 432535 6a6a79e2c19b5b08ffcef13fc0741f98ddcecf19
child 432537 37b4dc3bc73a31cb6d347b40fbf754e4c0b11761
push id106775
push usermasayuki@d-toybox.com
push dateTue, 21 Aug 2018 09:47:42 +0000
treeherdermozilla-inbound@4f0a12bcb401 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1484110
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1484110 - part 3: HTMLEditor::RefereshEditingUI() should refresh UIs when one of them is changed to enabled or disabled r=m_kato HTMLEditor::RefereshEditingUI() works only with enabled UIs. Therefore, if UI is disabled while it's visible, it keeps shown. This is too bad if web apps tries to disable the Gecko specific UIs after we show some of them. This patch adds HTMLEditor::HideAnonymousEditingUIsIfUnnecessary() to hide unnecessary UIs and makes RefereshEditingUI() call it always.
editor/libeditor/HTMLAnonymousNodeEditor.cpp
editor/libeditor/HTMLEditor.cpp
editor/libeditor/HTMLEditor.h
editor/libeditor/tests/test_abs_positioner_appearance.html
editor/libeditor/tests/test_resizers_appearance.html
--- a/editor/libeditor/HTMLAnonymousNodeEditor.cpp
+++ b/editor/libeditor/HTMLAnonymousNodeEditor.cpp
@@ -277,32 +277,81 @@ HTMLEditor::DeleteRefToAnonymousNode(Man
     // FIXME(emilio): This is the only caller to PresShell::ContentRemoved that
     // passes NAC into it. This is not great!
     aShell->ContentRemoved(aContent, nullptr);
   }
 
   // The ManualNACPtr destructor will invoke UnbindFromTree.
 }
 
+void
+HTMLEditor::HideAnonymousEditingUIs()
+{
+  if (mAbsolutelyPositionedObject) {
+    HideGrabber();
+    NS_ASSERTION(!mAbsolutelyPositionedObject, "HideGrabber failed");
+  }
+  if (mInlineEditedCell) {
+    HideInlineTableEditingUI();
+    NS_ASSERTION(!mInlineEditedCell, "HideInlineTableEditingUI failed");
+  }
+  if (mResizedObject) {
+    DebugOnly<nsresult> rv = HideResizers();
+    NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "HideResizers() failed");
+    NS_ASSERTION(!mResizedObject, "HideResizers failed");
+  }
+}
+
+void
+HTMLEditor::HideAnonymousEditingUIsIfUnnecessary()
+{
+  // XXX Perhaps, this is wrong approach to hide multiple UIs because
+  //     hiding one UI may causes overwriting existing UI with newly
+  //     created one.  In such case, we will leak ovewritten UI.
+  if (!IsAbsolutePositionEditorEnabled() && mAbsolutelyPositionedObject) {
+    // XXX If we're moving something, we need to cancel or commit the
+    //     operation now.
+    HideGrabber();
+    NS_ASSERTION(!mAbsolutelyPositionedObject, "HideGrabber failed");
+  }
+  if (!IsInlineTableEditorEnabled() && mInlineEditedCell) {
+    // XXX If we're resizing a table element, we need to cancel or commit the
+    //     operation now.
+    HideInlineTableEditingUI();
+    NS_ASSERTION(!mInlineEditedCell, "HideInlineTableEditingUI failed");
+  }
+  if (!IsObjectResizerEnabled() && mResizedObject) {
+    // XXX If we're resizing something, we need to cancel or commit the
+    //     operation now.
+    DebugOnly<nsresult> rv = HideResizers();
+    NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "HideResizers() failed");
+    NS_ASSERTION(!mResizedObject, "HideResizers failed");
+  }
+}
+
 NS_IMETHODIMP
 HTMLEditor::CheckSelectionStateForAnonymousButtons(Selection* aSelection)
 {
   if (NS_WARN_IF(!aSelection)) {
     return NS_ERROR_INVALID_ARG;
   }
   nsresult rv = RefereshEditingUI(*aSelection);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
   return NS_OK;
 }
 
 nsresult
 HTMLEditor::RefereshEditingUI(Selection& aSelection)
 {
+  // First, we need to remove unnecessary editing UI now since some of them
+  // may be disabled while them are visible.
+  HideAnonymousEditingUIsIfUnnecessary();
+
   // early way out if all contextual UI extensions are disabled
   if (!IsObjectResizerEnabled() &&
       !IsAbsolutePositionEditorEnabled() &&
       !IsInlineTableEditorEnabled()) {
     return NS_OK;
   }
 
   // Don't change selection state if we're moving.
@@ -382,17 +431,17 @@ HTMLEditor::RefereshEditingUI(Selection&
     // anything.  So, it's okay for now.
     nsresult rv = HideResizers();
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
     NS_ASSERTION(!mResizedObject, "HideResizers failed");
   }
 
-  if (mIsInlineTableEditingEnabled && mInlineEditedCell &&
+  if (IsInlineTableEditorEnabled() && mInlineEditedCell &&
       mInlineEditedCell != cellElement) {
     // XXX HideInlineTableEditingUI() won't return error.  Should be change it
     //     void later.
     HideInlineTableEditingUI();
     NS_ASSERTION(!mInlineEditedCell, "HideInlineTableEditingUI failed");
   }
 
   // now, let's display all contextual UI for good
@@ -426,17 +475,17 @@ HTMLEditor::RefereshEditingUI(Selection&
     } else {
       nsresult rv = ShowGrabber(*absPosElement);
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
     }
   }
 
-  if (mIsInlineTableEditingEnabled && cellElement &&
+  if (IsInlineTableEditorEnabled() && cellElement &&
       IsModifiableNode(*cellElement) && cellElement != hostContent) {
     if (mInlineEditedCell) {
       nsresult rv = RefreshInlineTableEditingUI();
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
     } else {
       nsresult rv = ShowInlineTableEditingUI(cellElement);
--- a/editor/libeditor/HTMLEditor.cpp
+++ b/editor/libeditor/HTMLEditor.cpp
@@ -253,30 +253,16 @@ HTMLEditor::~HTMLEditor()
     mHasShownInlineTableEditor ? 1 : 0);
   if (mHasShownInlineTableEditor) {
     Telemetry::Accumulate(
       Telemetry::HTMLEDITORS_WHOSE_INLINE_TABLE_EDITOR_USED_BY_USER,
       mInlineTableEditorUsedCount);
   }
 }
 
-void
-HTMLEditor::HideAnonymousEditingUIs()
-{
-  if (mAbsolutelyPositionedObject) {
-    HideGrabber();
-  }
-  if (mInlineEditedCell) {
-    HideInlineTableEditingUI();
-  }
-  if (mResizedObject) {
-    HideResizers();
-  }
-}
-
 NS_IMPL_CYCLE_COLLECTION_CLASS(HTMLEditor)
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(HTMLEditor, TextEditor)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mTypeInState)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mComposerCommandsUpdater)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mStyleSheets)
 
   tmp->HideAnonymousEditingUIs();
--- a/editor/libeditor/HTMLEditor.h
+++ b/editor/libeditor/HTMLEditor.h
@@ -222,44 +222,68 @@ public:
   nsresult OnMouseMove(dom::MouseEvent* aMouseEvent);
 
   /**
    * Enable/disable object resizers for <img> elements, <table> elements,
    * absolute positioned elements (required absolute position editor enabled).
    */
   void EnableObjectResizer(bool aEnable)
   {
+    if (mIsObjectResizingEnabled == aEnable) {
+      return;
+    }
     mIsObjectResizingEnabled = aEnable;
+    RefPtr<Selection> selection = GetSelection();
+    if (NS_WARN_IF(!selection)) {
+      return;
+    }
+    RefereshEditingUI(*selection);
   }
   bool IsObjectResizerEnabled() const
   {
     return mIsObjectResizingEnabled;
   }
 
   /**
    * Enable/disable inline table editor, e.g., adding new row or column,
    * removing existing row or column.
    */
   void EnableInlineTableEditor(bool aEnable)
   {
+    if (mIsInlineTableEditingEnabled == aEnable) {
+      return;
+    }
     mIsInlineTableEditingEnabled = aEnable;
+    RefPtr<Selection> selection = GetSelection();
+    if (NS_WARN_IF(!selection)) {
+      return;
+    }
+    RefereshEditingUI(*selection);
   }
   bool IsInlineTableEditorEnabled() const
   {
     return mIsInlineTableEditingEnabled;
   }
 
   /**
    * Enable/disable absolute position editor, resizing absolute positioned
    * elements (required object resizers enabled) or positioning them with
    * dragging grabber.
    */
   void EnableAbsolutePositionEditor(bool aEnable)
   {
+    if (mIsAbsolutelyPositioningEnabled == aEnable) {
+      return;
+    }
     mIsAbsolutelyPositioningEnabled = aEnable;
+    RefPtr<Selection> selection = GetSelection();
+    if (NS_WARN_IF(!selection)) {
+      return;
+    }
+    RefereshEditingUI(*selection);
   }
   bool IsAbsolutePositionEditorEnabled() const
   {
     return mIsAbsolutelyPositioningEnabled;
   }
 
   // non-virtual methods of interface methods
 
@@ -1572,19 +1596,30 @@ protected: // Shouldn't be used by frien
   int32_t GetNewResizingX(int32_t aX, int32_t aY);
   int32_t GetNewResizingY(int32_t aX, int32_t aY);
   int32_t GetNewResizingWidth(int32_t aX, int32_t aY);
   int32_t GetNewResizingHeight(int32_t aX, int32_t aY);
   void HideShadowAndInfo();
   void SetFinalSize(int32_t aX, int32_t aY);
   void SetResizeIncrements(int32_t aX, int32_t aY, int32_t aW, int32_t aH,
                            bool aPreserveRatio);
+
+  /**
+   * HideAnonymousEditingUIs() forcibly hides all editing UIs (resizers,
+   * inline-table-editing UI, absolute positioning UI).
+   */
   void HideAnonymousEditingUIs();
 
   /**
+   * HideAnonymousEditingUIsIfUnnecessary() hides all editing UIs if some of
+   * visible UIs are now unnecessary.
+   */
+  void HideAnonymousEditingUIsIfUnnecessary();
+
+  /**
    * sets the z-index of an element.
    * @param aElement [IN] the element
    * @param aZorder  [IN] the z-index
    */
   void SetZIndex(Element& aElement, int32_t aZorder);
 
   /**
    * shows a grabber attached to an arbitrary element. The grabber is an image
--- a/editor/libeditor/tests/test_abs_positioner_appearance.html
+++ b/editor/libeditor/tests/test_abs_positioner_appearance.html
@@ -73,16 +73,26 @@ SimpleTest.waitForFocus(async function()
       synthesizeMouseAtCenter(outOfEditor, {});
       let promiseSelectionChangeEvent2 = waitForSelectionChange();
       synthesizeMouseAtCenter(target, {});
       await promiseSelectionChangeEvent2;
 
       is(target.hasAttribute("_moz_abspos"), kTest.movable,
          kDescription + (kTest.movable ? "While enableAbsolutePositionEditing is enabled, positioner should appear" :
                                          "Even while enableAbsolutePositionEditing is enabled, positioner shouldn't appear"));
+
+      document.execCommand("enableAbsolutePositionEditing", false, false);
+      ok(!target.hasAttribute("_moz_abspos"),
+         kDescription + "When enableAbsolutePositionEditing is disabled even while positioner is visible, positioner should disappear");
+
+      document.execCommand("enableAbsolutePositionEditing", false, true);
+      is(target.hasAttribute("_moz_abspos"), kTest.movable,
+         kDescription + (kTest.movable ?
+                           "When enableAbsolutePositionEditing is enabled when absolute positioned element is selected, positioner should appear" :
+                           "Even if enableAbsolutePositionEditing is enabled when static positioned element is selected, positioner shouldn't appear"));
     }
   }
 
   async function testStyle() {
     // See HTMLEditor::GetTemporaryStyleForFocusedPositionedElement().
     const kTests = [
       { description: "background-color: transparent; color: white;",
         innerHTML: "<div id=\"target\" style=\"position: absolute; " +
--- a/editor/libeditor/tests/test_resizers_appearance.html
+++ b/editor/libeditor/tests/test_resizers_appearance.html
@@ -84,16 +84,25 @@ SimpleTest.waitForFocus(async function()
       let promiseSelectionChangeEvent2 = waitForSelectionChange();
       synthesizeMouseAtCenter(target, {});
       await promiseSelectionChangeEvent2;
 
       const kResizable = typeof kTest.resizable === "function" ? kTest.resizable() : kTest.resizable;
       is(target.hasAttribute("_moz_resizing"), kResizable,
          kDescription + (kResizable ? "While enableObjectResizing is enabled, resizers should appear" :
                                       "Even while enableObjectResizing is enabled, resizers shouldn't appear"));
+
+      document.execCommand("enableObjectResizing", false, false);
+      ok(!target.hasAttribute("_moz_resizing"),
+         kDescription + "enableObjectResizing is disabled even while resizers are visible, resizers should disappear");
+
+      document.execCommand("enableObjectResizing", false, true);
+      is(target.hasAttribute("_moz_resizing"), kResizable,
+         kDescription + (kResizable ? "enableObjectResizing is enabled when resizable object is selected, resizers should appear" :
+                                      "Even if enableObjectResizing is enabled when non-resizable object is selected, resizers shouldn't appear"));
     }
   }
 
   SimpleTest.finish();
 });
 </script>
 </pre>
 </body>