Bug 1551185 - Make nsImageFrame::ShouldDisplaySelection() check whether resizer target element is its content or not r=smaug

Currently, `nsISelectionDisplay::DISPLAY_ALL` is used only by `HTMLEditor`.
And only when it's set, `nsImageFrame::ShouldDisplaySelection()` returns `false`
if only its `mContent` is selected.  However, this is based on an assumption,
that is, when only one `<img>` is selected in an HTML editor, it's target of
resizers.  However, this is completely wrong.  Web apps can disable resizers
with `document.execCommand("enableObjectResizing", false, false)` and now,
it's disabled by default.

Therefore, this patch makes the method check whether its `mContent` is
target of resizers at the moment.

Differential Revision: https://phabricator.services.mozilla.com/D32449

--HG--
extra : moz-landing-system : lando

editor/libeditor/HTMLEditor.h

@@ -268,6 +268,8 @@ class HTMLEditor final : public TextEditor,
   }
   bool IsObjectResizerEnabled() const { return mIsObjectResizingEnabled; }
 
+  Element* GetResizerTarget() const { return mResizedObject; }
+
   /**
    * Enable/disable inline table editor, e.g., adding new row or column,
    * removing existing row or column.

layout/generic/nsImageFrame.cpp

@@ -2128,30 +2128,23 @@ void nsImageFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
 
 bool nsImageFrame::ShouldDisplaySelection() {
   int16_t displaySelection = PresShell()->GetSelectionFlags();
-  if (!(displaySelection & nsISelectionDisplay::DISPLAY_IMAGES))
-    return false;  // no need to check the blue border, we cannot be drawn
-                   // selected
-
-  // If the image is the only selected node, don't draw the selection overlay.
-  // This can happen when selecting an image in contenteditable context.
-  if (displaySelection == nsISelectionDisplay::DISPLAY_ALL) {
-    if (const nsFrameSelection* frameSelection = GetConstFrameSelection()) {
-      const Selection* selection =
-          frameSelection->GetSelection(SelectionType::eNormal);
-      if (selection && selection->RangeCount() == 1) {
-        nsINode* parent = mContent->GetParent();
-        int32_t thisOffset = parent->ComputeIndexOf(mContent);
-        nsRange* range = selection->GetRangeAt(0);
-        if (range->GetStartContainer() == parent &&
-            range->GetEndContainer() == parent &&
-            static_cast<int32_t>(range->StartOffset()) == thisOffset &&
-            static_cast<int32_t>(range->EndOffset()) == thisOffset + 1) {
+  if (!(displaySelection & nsISelectionDisplay::DISPLAY_IMAGES)) {
+    // no need to check the blue border, we cannot be drawn selected.
     return false;
   }
-      }
-    }
-  }
+
+  if (displaySelection != nsISelectionDisplay::DISPLAY_ALL) {
     return true;
+  }
+
+  // If the image is currently resize target of the editor, don't draw the
+  // selection overlay.
+  HTMLEditor* htmlEditor = nsContentUtils::GetHTMLEditor(PresContext());
+  if (!htmlEditor) {
+    return true;
+  }
+
+  return htmlEditor->GetResizerTarget() != mContent;
 }
 
 nsImageMap* nsImageFrame::GetImageMap() {

layout/generic/test/mochitest.ini

@@ -105,6 +105,7 @@ skip-if = os=='win'
 [test_dynamic_reflow_root_disallowal.html]
 [test_image_selection.html]
 [test_image_selection_2.html]
+[test_image_selection_in_contenteditable.html]
 [test_invalidate_during_plugin_paint.html]
 skip-if = toolkit == 'android' || headless # Headless:Bug 1405871
 [test_intrinsic_size_on_loading.html]

layout/generic/test/test_image_selection.html

@@ -73,31 +73,17 @@ function step4() {
 
   gFuchsiaSelected = snapshotWindow(gIframe.contentWindow, false);
 
-  if (gIframe.contentDocument.queryCommandState("enableObjectResizing")) {
-    assert_different(gBlueNotSelected, gBlueSelected,
-                     "selecting image should add drag points");
-  } else {
-    assert_equal(gBlueNotSelected, gBlueSelected,
-                 "selecting image should not change anything visually");
-  }
-  assert_different(gBlueSelected, gFuchsiaSelected,
-                   "different images should appear different");
+  // FYI: test_image_selection_in_contenteditable.html tests the detail.
+  assertSnapshots(gBlueNotSelected, gBlueSelected, false, null,
+                  "blue image without selection",
+                  "blue image which is selected or added resizers");
+  assertSnapshots(gBlueSelected, gFuchsiaSelected, false, null,
+                  "blue image which is selected",
+                  "fuchsia image which is selected");
 
   SimpleTest.finish();
 }
 
-function assert_equal(shot1, shot2, desc)
-{
-  var [correct, s1, s2] = compareSnapshots(shot1, shot2, true);
-  ok(correct, desc + (correct ? "" : "\nRESULT: " + s2));
-}
-
-function assert_different(shot1, shot2, desc)
-{
-  var [correct, s1, s2] = compareSnapshots(shot1, shot2, false);
-  ok(correct, desc);
-}
-
 </script>
 </pre>
 </body>

layout/generic/test/test_image_selection_in_contenteditable.html

@@ -0,0 +1,85 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1551185
+-->
+  <head>
+    <meta charset="utf-8">
+    <title>Test for Bug 1551185</title>
+    <script src="/tests/SimpleTest/SimpleTest.js"></script>
+    <script src="/tests/SimpleTest/WindowSnapshot.js"></script>
+    <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+    <style>
+      #editor:focus {
+        outline: none;
+      }
+    </style>
+  </head>
+  <body>
+    <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1551185">Mozilla Bug 1551185</a>
+    <p id="display"></p>
+    <div id="editor"><img id="img1" src="blue-32x32.png"><img id="img2" src="blue-32x32.png"></div>
+    <div id="content" style="display: none"></div>
+    <pre id="test">
+      <script type="application/javascript">
+        SimpleTest.waitForExplicitFinish();
+        SimpleTest.waitForFocus(() => {
+          let selection = window.getSelection();
+          let selectionController =
+              SpecialPowers.wrap(window)
+                           .docShell
+                           .QueryInterface(SpecialPowers.Ci.nsIInterfaceRequestor)
+                           .getInterface(SpecialPowers.Ci.nsISelectionDisplay)
+                           .QueryInterface(SpecialPowers.Ci.nsISelectionController);
+          let editor = document.getElementById("editor");
+          let img1 = document.getElementById("img1");
+          let img2 = document.getElementById("img2");
+          // First, take snapshot without selection.
+          selection.removeAllRanges();
+          const kSnapshotWithoutSelection = snapshotWindow(window);
+          // Next, take snapshot of selected both images.
+          selection.setBaseAndExtent(editor, 0, editor, 2);
+          const kSnapshotBothImagesSelected = snapshotWindow(window);
+          // Next, take snapshot of selected the last image.
+          selection.setBaseAndExtent(editor, 1, editor, 2);
+          const kSnapshotLastImageSelected = snapshotWindow(window);
+
+          editor.contentEditable = true;
+          editor.focus();
+
+          document.execCommand("enableObjectResizing", false, false);
+          selection.collapse(editor, 2);
+          selectionController.characterMove(false, true);
+          // When object resizing is disabled, <img> elements should be rendered
+          // as selected if they are in selection ranges.
+          assertSnapshots(snapshotWindow(window), kSnapshotLastImageSelected, true, null,
+                          "Selects only the last image when object resizing is disabled", "");
+          selectionController.characterMove(false, true);
+          assertSnapshots(snapshotWindow(window), kSnapshotBothImagesSelected, true, null,
+                          "Selects both the images when object resizing is disabled", "");
+
+          document.execCommand("enableObjectResizing", false, true);
+          selection.collapse(editor, 2);
+          // When an <img> element is selected, it should become target of resizers.
+          // At this time, the image shouldn't be rendered as selected but there should
+          // be resizers.  I.e., shouldn't match with kSnapshotWithoutSelection nor
+          // kSnapshotLastImageSelected.
+          selectionController.characterMove(false, true);
+          assertSnapshots(snapshotWindow(window), kSnapshotLastImageSelected, false, null,
+                          "Selects only the last image when object resizing is enabled",
+                          "(compared with the last image selected snapshot)");
+          assertSnapshots(snapshotWindow(window), kSnapshotWithoutSelection, false, null,
+                          "Selects only the last image when object resizing is enabled",
+                          "(compared without no selection)");
+          // If not only one <img> element is selected, selected <img> elements should
+          // be rendered as selected normally.
+          selectionController.characterMove(false, true);
+          assertSnapshots(snapshotWindow(window), kSnapshotBothImagesSelected, true, null,
+                          "Selects both the images when object resizing is enabled");
+
+          SimpleTest.finish();
+        });
+      </script>
+    </pre>
+  </body>
+</html>