Bug 1320815 - DeCOMtaminate nsIFrame::IsSelectable by returning boolean instead of nsresult. r?xidorn draft
authorL. David Baron <dbaron@dbaron.org>
Mon, 28 Nov 2016 15:31:29 -0800
changeset 444878 a69b688bffa48696fd65177d61146561d31db6bb
parent 444877 60d40894e16323b11a7ce3a3082aa485f005cc73
child 538409 c5eb9c3bb62250d443b5776da475dd890fe55efc
push id37385
push userdbaron@mozilla.com
push dateMon, 28 Nov 2016 23:37:47 +0000
reviewersxidorn
bugs1320815
milestone53.0a1
Bug 1320815 - DeCOMtaminate nsIFrame::IsSelectable by returning boolean instead of nsresult. r?xidorn MozReview-Commit-ID: EBxBcEgIvp7
dom/base/nsDocumentEncoder.cpp
dom/base/nsRange.cpp
layout/base/AccessibleCaretManager.cpp
layout/generic/nsFrame.cpp
layout/generic/nsIFrame.h
layout/generic/nsTextFrame.cpp
--- a/dom/base/nsDocumentEncoder.cpp
+++ b/dom/base/nsDocumentEncoder.cpp
@@ -445,19 +445,17 @@ nsDocumentEncoder::SerializeToStringRecu
   if (!maybeFixedNode)
     maybeFixedNode = aNode;
 
   if ((mFlags & SkipInvisibleContent) &&
       !(mFlags & OutputNonTextContentAsPlaceholder)) {
     if (aNode->IsNodeOfType(nsINode::eCONTENT)) {
       nsIFrame* frame = static_cast<nsIContent*>(aNode)->GetPrimaryFrame();
       if (frame) {
-        bool isSelectable;
-        frame->IsSelectable(&isSelectable, nullptr);
-        if (!isSelectable){
+        if (!frame->IsSelectable(nullptr)) {
           aDontSerializeRoot = true;
         }
       }
     }
   }
 
   if (!aDontSerializeRoot) {
     int32_t endOffset = -1;
--- a/dom/base/nsRange.cpp
+++ b/dom/base/nsRange.cpp
@@ -3255,17 +3255,17 @@ nsRange::ExcludeNonSelectableNodes(nsTAr
           selectable = false;
         }
         if (selectable) {
           nsIFrame* frame = content->GetPrimaryFrame();
           for (nsIContent* p = content; !frame && (p = p->GetParent()); ) {
             frame = p->GetPrimaryFrame();
           }
           if (frame) {
-            frame->IsSelectable(&selectable, nullptr);
+            selectable = frame->IsSelectable(nullptr);
           }
         }
       }
 
       if (!selectable) {
         if (!firstNonSelectableContent) {
           firstNonSelectableContent = content;
         }
--- a/layout/base/AccessibleCaretManager.cpp
+++ b/layout/base/AccessibleCaretManager.cpp
@@ -609,18 +609,17 @@ AccessibleCaretManager::SelectWordOrShor
     }
     // We need to update carets to get correct information before dispatching
     // CaretStateChangedEvent.
     UpdateCaretsWithHapticFeedback();
     DispatchCaretStateChangedEvent(CaretChangedReason::Longpressonemptycontent);
     return NS_OK;
   }
 
-  bool selectable = false;
-  ptFrame->IsSelectable(&selectable, nullptr);
+  bool selectable = ptFrame->IsSelectable(nullptr);
 
 #ifdef DEBUG_FRAME_DUMP
   AC_LOG("%s: %s %s selectable.", __FUNCTION__, ptFrame->ListTag().get(),
          selectable ? "is" : "is NOT");
 #endif
 
   if (!selectable) {
     return NS_ERROR_FAILURE;
@@ -1206,19 +1205,17 @@ AccessibleCaretManager::DragCaretInterna
   nsPoint ptInFrame = point;
   nsLayoutUtils::TransformPoint(rootFrame, ptFrame, ptInFrame);
   result = fs->ConstrainFrameAndPointToAnchorSubtree(ptFrame, ptInFrame,
                                                      &newFrame, newPoint);
   if (NS_FAILED(result) || !newFrame) {
     return NS_ERROR_FAILURE;
   }
 
-  bool selectable;
-  newFrame->IsSelectable(&selectable, nullptr);
-  if (!selectable) {
+  if (!newFrame->IsSelectable(nullptr)) {
     return NS_ERROR_FAILURE;
   }
 
   nsIFrame::ContentOffsets offsets =
     newFrame->GetContentOffsetsFromPoint(newPoint);
   if (offsets.IsNull()) {
     return NS_ERROR_FAILURE;
   }
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -1721,19 +1721,17 @@ nsFrame::DisplaySelection(nsPresContext*
   int16_t selType = nsISelectionController::SELECTION_OFF;
 
   nsCOMPtr<nsISelectionController> selCon;
   nsresult result = GetSelectionController(aPresContext, getter_AddRefs(selCon));
   if (NS_SUCCEEDED(result) && selCon) {
     result = selCon->GetDisplaySelection(&selType);
     if (NS_SUCCEEDED(result) && (selType != nsISelectionController::SELECTION_OFF)) {
       // Check whether style allows selection.
-      bool selectable;
-      IsSelectable(&selectable, nullptr);
-      if (!selectable) {
+      if (!IsSelectable(nullptr)) {
         selType = nsISelectionController::SELECTION_OFF;
         isOkToTurnOn = false;
       }
     }
     if (isOkToTurnOn && (selType == nsISelectionController::SELECTION_OFF)) {
       selCon->SetDisplaySelection(nsISelectionController::SELECTION_ON);
       selType = nsISelectionController::SELECTION_ON;
     }
@@ -3158,21 +3156,20 @@ nsFrame::GetDataForTableSelection(const 
   if (foundCell)
     *aTarget = nsISelectionPrivate::TABLESELECTION_CELL;
   else if (foundTable)
     *aTarget = nsISelectionPrivate::TABLESELECTION_TABLE;
 
   return NS_OK;
 }
 
-nsresult
-nsIFrame::IsSelectable(bool* aSelectable, StyleUserSelect* aSelectStyle) const
-{
-  if (!aSelectable) //it's ok if aSelectStyle is null
-    return NS_ERROR_NULL_POINTER;
+bool
+nsIFrame::IsSelectable(StyleUserSelect* aSelectStyle) const
+{
+  // it's ok if aSelectStyle is null
 
   // Like 'visibility', we must check all the parents: if a parent
   // is not selectable, none of its children is selectable.
   //
   // The -moz-all value acts similarly: if a frame has 'user-select:-moz-all',
   // all its children are selectable, even those with 'user-select:none'.
   //
   // As a result, if 'none' and '-moz-all' are not present in the frame hierarchy,
@@ -3234,23 +3231,19 @@ nsIFrame::IsSelectable(bool* aSelectable
     allowSelection = !containsEditable;
   }
 
   // return stuff
   if (aSelectStyle) {
     *aSelectStyle = selectStyle;
   }
 
-  if (mState & NS_FRAME_GENERATED_CONTENT) {
-    *aSelectable = false;
-  } else {
-    *aSelectable = allowSelection && (selectStyle != StyleUserSelect::None);
-  }
-
-  return NS_OK;
+  return !(mState & NS_FRAME_GENERATED_CONTENT) &&
+         allowSelection &&
+         selectStyle != StyleUserSelect::None;
 }
 
 /**
   * Handles the Mouse Press Event for the frame
  */
 NS_IMETHODIMP
 nsFrame::HandlePress(nsPresContext* aPresContext, 
                      WidgetGUIEvent* aEvent,
@@ -3267,17 +3260,16 @@ nsFrame::HandlePress(nsPresContext* aPre
   }
 
   //We often get out of sync state issues with mousedown events that
   //get interrupted by alerts/dialogs.
   //Check with the ESM to see if we should process this one
   if (!aPresContext->EventStateManager()->EventStatusOK(aEvent)) 
     return NS_OK;
 
-  nsresult rv;
   nsIPresShell *shell = aPresContext->GetPresShell();
   if (!shell)
     return NS_ERROR_FAILURE;
 
   // if we are in Navigator and the click is in a draggable node, we don't want
   // to start selection because we don't want to interfere with a potential
   // drag of said node and steal all its glory.
   int16_t isEditor = shell->GetSelectionFlags();
@@ -3297,24 +3289,21 @@ nsFrame::HandlePress(nsPresContext* aPre
           return NS_OK;
         }
       }
     }
   }
 
   // check whether style allows selection
   // if not, don't tell selection the mouse event even occurred.  
-  bool    selectable;
   StyleUserSelect selectStyle;
-  rv = IsSelectable(&selectable, &selectStyle);
-  if (NS_FAILED(rv)) return rv;
-  
   // check for select: none
-  if (!selectable)
+  if (!IsSelectable(&selectStyle)) {
     return NS_OK;
+  }
 
   // When implementing StyleUserSelect::Element, StyleUserSelect::Elements and
   // StyleUserSelect::Toggle, need to change this logic
   bool useFrameSelection = (selectStyle == StyleUserSelect::Text);
 
   // If the mouse is dragged outside the nearest enclosing scrollable area
   // while making a selection, the area will be scrolled. To do this, capture
   // the mouse on the nearest scrollable frame. If there isn't a scrollable
@@ -3385,16 +3374,17 @@ nsFrame::HandlePress(nsPresContext* aPre
                            offsets.EndOffset(), false, false,
                            offsets.associate);
   }
 
   // Let Ctrl/Cmd+mouse down do table selection instead of drag initiation
   nsCOMPtr<nsIContent>parentContent;
   int32_t  contentOffset;
   int32_t target;
+  nsresult rv;
   rv = GetDataForTableSelection(frameselection, shell, mouseEvent,
                                 getter_AddRefs(parentContent), &contentOffset,
                                 &target);
   if (NS_SUCCEEDED(rv) && parentContent)
   {
     fc->SetDragState(true);
     return fc->HandleTableSelection(parentContent, contentOffset, target,
                                     mouseEvent);
@@ -7437,20 +7427,17 @@ nsFrame::GetNextPrevLineFromeBlockFrame(
           resultFrame->GetOffsetFromView(offset, &view);
           ContentOffsets offsets =
               resultFrame->GetContentOffsetsFromPoint(point - offset);
           aPos->mResultContent = offsets.content;
           aPos->mContentOffset = offsets.offset;
           aPos->mAttach = offsets.associate;
           if (offsets.content)
           {
-            bool selectable;
-            resultFrame->IsSelectable(&selectable, nullptr);
-            if (selectable)
-            {
+            if (resultFrame->IsSelectable(nullptr)) {
               found = true;
               break;
             }
           }
         }
 
         if (aPos->mDirection == eDirPrevious && (resultFrame == farStoppingFrame))
           break;
@@ -7482,20 +7469,17 @@ nsFrame::GetNextPrevLineFromeBlockFrame(
         resultFrame->GetOffsetFromView(offset, &view);
         ContentOffsets offsets =
             resultFrame->GetContentOffsetsFromPoint(point - offset);
         aPos->mResultContent = offsets.content;
         aPos->mContentOffset = offsets.offset;
         aPos->mAttach = offsets.associate;
         if (offsets.content)
         {
-          bool selectable;
-          resultFrame->IsSelectable(&selectable, nullptr);
-          if (selectable)
-          {
+          if (resultFrame->IsSelectable(nullptr)) {
             found = true;
             if (resultFrame == farStoppingFrame)
               aPos->mAttach = CARET_ASSOCIATE_BEFORE;
             else
               aPos->mAttach = CARET_ASSOCIATE_AFTER;
             break;
           }
         }
@@ -8296,17 +8280,17 @@ nsIFrame::GetFrameFromDirection(nsDirect
       if (NS_FAILED(result)) {
         return result;
       }
       if (lineFrameCount > 1) {
         continue;
       }
     }
 
-    traversedFrame->IsSelectable(&selectable, nullptr);
+    selectable = traversedFrame->IsSelectable(nullptr);
     if (!selectable) {
       *aOutMovedOverNonSelectableText = true;
     }
   } // while (!selectable)
 
   *aOutOffset = (aDirection == eDirNext) ? 0 : -1;
 
   if (aVisual && IsReversedDirectionFrame(traversedFrame)) {
--- a/layout/generic/nsIFrame.h
+++ b/layout/generic/nsIFrame.h
@@ -2777,23 +2777,23 @@ public:
    * @returns true if this frame is selected.
    */
   bool IsSelected() const;
 
   /**
    *  called to discover where this frame, or a parent frame has user-select style
    *  applied, which affects that way that it is selected.
    *
-   *  @param aIsSelectable out param. Set to true if the frame can be selected
-   *                       (i.e. is not affected by user-select: none)
    *  @param aSelectStyle  out param. Returns the type of selection style found
    *                        (using values defined in nsStyleConsts.h).
+   *
+   *  @return Whether the frame can be selected (i.e. is not affected by
+   *          user-select: none)
    */
-  nsresult IsSelectable(bool* aIsSelectable,
-                        mozilla::StyleUserSelect* aSelectStyle) const;
+  bool IsSelectable(mozilla::StyleUserSelect* aSelectStyle) const;
 
   /** 
    *  Called to retrieve the SelectionController associated with the frame.
    *  @param aSelCon will contain the selection controller associated with
    *  the frame.
    */
   virtual nsresult  GetSelectionController(nsPresContext *aPresContext, nsISelectionController **aSelCon) = 0;
 
--- a/layout/generic/nsTextFrame.cpp
+++ b/layout/generic/nsTextFrame.cpp
@@ -4520,19 +4520,17 @@ nsTextFrame::~nsTextFrame()
 }
 
 nsresult
 nsTextFrame::GetCursor(const nsPoint& aPoint,
                        nsIFrame::Cursor& aCursor)
 {
   FillCursorInformationFromStyle(StyleUserInterface(), aCursor);
   if (NS_STYLE_CURSOR_AUTO == aCursor.mCursor) {
-    bool selectable;
-    IsSelectable(&selectable, nullptr);
-    if (!selectable) {
+    if (!IsSelectable(nullptr)) {
       aCursor.mCursor = NS_STYLE_CURSOR_DEFAULT;
     } else {
       aCursor.mCursor = GetWritingMode().IsVertical()
         ? NS_STYLE_CURSOR_VERTICAL_TEXT : NS_STYLE_CURSOR_TEXT;
     }
     return NS_OK;
   } else {
     return nsFrame::GetCursor(aPoint, aCursor);
@@ -7848,19 +7846,18 @@ IsAcceptableCaretPosition(const gfxSkipC
 
 nsIFrame::FrameSearchResult
 nsTextFrame::PeekOffsetCharacter(bool aForward, int32_t* aOffset,
                                  bool aRespectClusters)
 {
   int32_t contentLength = GetContentLength();
   NS_ASSERTION(aOffset && *aOffset <= contentLength, "aOffset out of range");
 
-  bool selectable;
   StyleUserSelect selectStyle;
-  IsSelectable(&selectable, &selectStyle);
+  IsSelectable(&selectStyle);
   if (selectStyle == StyleUserSelect::All)
     return CONTINUE_UNSELECTABLE;
 
   gfxSkipCharsIterator iter = EnsureTextRun(nsTextFrame::eInflated);
   if (!mTextRun)
     return CONTINUE_EMPTY;
 
   TrimmedOffsets trimmed = GetTrimmedOffsets(mContent->GetText(), false);
@@ -8037,19 +8034,18 @@ ClusterIterator::ClusterIterator(nsTextF
 
 nsIFrame::FrameSearchResult
 nsTextFrame::PeekOffsetWord(bool aForward, bool aWordSelectEatSpace, bool aIsKeyboardSelect,
                             int32_t* aOffset, PeekWordState* aState)
 {
   int32_t contentLength = GetContentLength();
   NS_ASSERTION (aOffset && *aOffset <= contentLength, "aOffset out of range");
 
-  bool selectable;
   StyleUserSelect selectStyle;
-  IsSelectable(&selectable, &selectStyle);
+  IsSelectable(&selectStyle);
   if (selectStyle == StyleUserSelect::All)
     return CONTINUE_UNSELECTABLE;
 
   int32_t offset = GetContentOffset() + (*aOffset < 0 ? contentLength : *aOffset);
   ClusterIterator cIter(this, offset, aForward ? 1 : -1, aState->mContext);
 
   if (!cIter.NextCluster())
     return CONTINUE_EMPTY;