Bug 1533293 - part 4: Make Selection::SetStartAndEndInternal() select frames by itself r=smaug
authorMasayuki Nakano <masayuki@d-toybox.com>
Tue, 26 Mar 2019 10:16:20 +0000
changeset 466082 e75b6b732e7c02670ec3cb2b18a4679216baeec8
parent 466081 1b38317f4237a11d18276e39567819b3f493c547
child 466083 dca8a22066e8ced5921ee52bec2719fba8a70c70
push id81426
push usermasayuki@d-toybox.com
push dateTue, 26 Mar 2019 10:19:05 +0000
treeherderautoland@e75b6b732e7c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1533293
milestone68.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 1533293 - part 4: Make Selection::SetStartAndEndInternal() select frames by itself r=smaug When adding an range via `Selection::AddItem()` and only when it's caused by user's operation, the range may be split at non-selectable content. This is done in `nsRange::ExcludeNonSelectableNodes()` but it modifies the given range as the first range. Then, [`Selection::AddRangeInternal()` calls `Selection::SelectFrames()` with the range which may have been modified by `nsRange::ExcludeNonSelectableNodes()`](https://searchfox.org/mozilla-central/rev/358f816f63da072145c593e9e2ac36b7250ecd25/dom/base/Selection.cpp#2030,2049). Therefore, only the frames between first child of `<body>` element and previous element of first non-selectable content are painted as selection after user does "Select All". [`Selection::Extend()` calls `Selection::SelectFrames()` by itself for all existing ranges](https://searchfox.org/mozilla-central/rev/358f816f63da072145c593e9e2ac36b7250ecd25/dom/base/Selection.cpp#2718-2724). Therefore, this patch creates new method and makes both `Selection::Extend()` and `Selection::SetStartAndEndInternal()` call it only when the result is not only one range. Differential Revision: https://phabricator.services.mozilla.com/D24871
dom/base/Selection.cpp
dom/base/Selection.h
--- a/dom/base/Selection.cpp
+++ b/dom/base/Selection.cpp
@@ -1504,16 +1504,24 @@ nsresult Selection::SelectAllFramesForCo
     MOZ_ASSERT(node);
     nsIContent* innercontent = node->IsContent() ? node->AsContent() : nullptr;
     SelectFramesForContent(innercontent, aSelected);
   }
 
   return NS_OK;
 }
 
+void Selection::SelectFramesInAllRanges(nsPresContext* aPresContext) {
+  for (size_t i = 0; i < mRanges.Length(); ++i) {
+    nsRange* range = mRanges[i].mRange;
+    MOZ_ASSERT(range->IsInSelection());
+    SelectFrames(aPresContext, range, range->IsInSelection());
+  }
+}
+
 /**
  * The idea of this helper method is to select or deselect "top to bottom",
  * traversing through the frames
  */
 nsresult Selection::SelectFrames(nsPresContext* aPresContext, nsRange* aRange,
                                  bool aSelect) {
   if (!mFrameSelection || !aPresContext || !aPresContext->GetPresShell()) {
     // nothing to do
@@ -2711,21 +2719,17 @@ void Selection::Extend(nsINode& aContain
     res = SetAnchorFocusToRange(range);
     if (NS_FAILED(res)) {
       aRv.Throw(res);
       return;
     }
   }
 
   if (mRanges.Length() > 1) {
-    for (size_t i = 0; i < mRanges.Length(); ++i) {
-      nsRange* range = mRanges[i].mRange;
-      MOZ_ASSERT(range->IsInSelection());
-      SelectFrames(presContext, range, range->IsInSelection());
-    }
+    SelectFramesInAllRanges(presContext);
   }
 
   DEBUG_OUT_RANGE(range);
 #ifdef DEBUG_SELECTION
   if (GetDirection() != oldDirection) {
     printf("    direction changed to %s\n",
            GetDirection() == eDirNext ? "eDirNext" : "eDirPrevious");
   }
@@ -3482,16 +3486,26 @@ void Selection::SetStartAndEndInternal(I
     return;
   }
 
   AddRange(*newRange, aRv);
   if (aRv.Failed()) {
     return;
   }
 
+  // Adding a range may set 2 or more ranges if there are non-selectable
+  // contents only when this change is caused by a user operation.  Therefore,
+  // we need to select frames with the result in such case.
+  if (mUserInitiated) {
+    RefPtr<nsPresContext> presContext = GetPresContext();
+    if (mRanges.Length() > 1 && presContext) {
+      SelectFramesInAllRanges(presContext);
+    }
+  }
+
   SetDirection(aDirection);
 }
 
 /** SelectionLanguageChange modifies the cursor Bidi level after a change in
  * keyboard direction
  *  @param aLangRTL is true if the new language is right-to-left or false if the
  * new language is left-to-right
  */
--- a/dom/base/Selection.h
+++ b/dom/base/Selection.h
@@ -693,16 +693,22 @@ class Selection final : public nsSupport
   void SetAnchorFocusRange(int32_t aIndex);
   void SelectFramesForContent(nsIContent* aContent, bool aSelected);
   nsresult SelectAllFramesForContent(PostContentIterator& aPostOrderIter,
                                      nsIContent* aContent, bool aSelected);
   nsresult SelectFrames(nsPresContext* aPresContext, nsRange* aRange,
                         bool aSelect);
 
   /**
+   * SelectFramesInAllRanges() calls SelectFrames() for all current
+   * ranges.
+   */
+  void SelectFramesInAllRanges(nsPresContext* aPresContext);
+
+  /**
    * Test whether the supplied range points to a single table element.
    * Result is one of the TableSelection constants. "None" means
    * a table element isn't selected.
    */
   nsresult GetTableSelectionType(nsRange* aRange,
                                  TableSelection* aTableSelectionType);
   nsresult GetTableCellLocationFromRange(nsRange* aRange,
                                          TableSelection* aSelectionType,