Commit a50a0700 authored by Masayuki Nakano's avatar Masayuki Nakano
Browse files

Bug 1530649 - Improve composition string handling which ends with whitespaces r=m_kato 

If insertion string ends with ASCII whitespace and there is no following
content in the block, `HTMLEditRules::AdjustWhitespaces()` needs to insert
`<br>` element.  It's called only by `HTMLEditRules::AfterEditInner()` and
that does only simple things with `WSRunObject`.  Therefore, this moves the
code into `AfterEditInner()`.

For making it adjust the whitespaces, `HTMLEditRules::WillInsertText()` needs
to notify `AfterEditInner()` of dirty range with `mDocChangeRange`.  Therefore,
this patch makes it set `mDocChangeRange` manually after inserting composition
string.

On the other hand, there is another bug.  `WSRunObject` was designed to treat
only inserting text for `WSRunObject::InsertText()`.  I.e., not designed to
treat replacing existing composition string with new string.  Therefore,
`WSRunObject::InsertText()` adjusts whitespaces only around start of
composition string.  Therefore, if composition string ends with an ASCII
whitespace, it's not replaced with NBSP and that causes:
- failing `WSRunObject::AdjustWhitespaces()` inserts `<br>` element at
  `AfterEditInner()` of committing composition.
- then, next composition's first `WSRunObject::InsertText()` removes the
  last whitespace due to not followed by `<br>` nor any other content.
Therefore, this patch makes `WSRunObject` takes 2 DOM points to be able to
treat replaced range.

In strictly speaking, the latter change require more changes and tests for
supporting replacement with any other methods.  However, it's risky and out
of scope of this bug.

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

--HG--
extra : moz-landing-system : lando
parent 8921690f

dom/events/TextComposition.cpp

+117 −0
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 
#include "mozilla/MiscEvents.h"

#include "mozilla/Preferences.h"

#include "mozilla/PresShell.h"

#include "mozilla/RangeBoundary.h"

#include "mozilla/StaticPrefs.h"

#include "mozilla/TextComposition.h"

#include "mozilla/TextEvents.h"
@@ -667,6 +668,122 @@ bool TextComposition::HasEditor() const { 
  return mEditorBaseWeak && mEditorBaseWeak->IsAlive();

}



RawRangeBoundary TextComposition::GetStartRef() const {

  RefPtr<EditorBase> editorBase = GetEditorBase();

  if (!editorBase) {

    return RawRangeBoundary();

  }



  nsISelectionController* selectionController =

      editorBase->GetSelectionController();

  if (NS_WARN_IF(!selectionController)) {

    return RawRangeBoundary();

  }



  nsRange* firstRange = nullptr;

  static const SelectionType kIMESelectionTypes[] = {

      SelectionType::eIMERawClause, SelectionType::eIMESelectedRawClause,

      SelectionType::eIMEConvertedClause, SelectionType::eIMESelectedClause};

  for (auto selectionType : kIMESelectionTypes) {

    Selection* selection =

        selectionController->GetSelection(ToRawSelectionType(selectionType));

    if (!selection) {

      continue;

    }

    for (uint32_t i = 0; i < selection->RangeCount(); i++) {

      nsRange* range = selection->GetRangeAt(i);

      if (NS_WARN_IF(!range) || NS_WARN_IF(!range->GetStartContainer())) {

        continue;

      }

      if (!firstRange) {

        firstRange = range;

        continue;

      }

      // In most cases, all composition string should be in same text node.

      if (firstRange->GetStartContainer() == range->GetStartContainer()) {

        if (firstRange->StartOffset() > range->StartOffset()) {

          firstRange = range;

        }

        continue;

      }

      // However, if web apps have inserted different nodes in composition

      // string, composition string may span 2 or more nodes.

      if (firstRange->GetStartContainer()->GetNextSibling() ==

          range->GetStartContainer()) {

        // Fast path for some known applications like Google Keep.

        firstRange = range;

        continue;

      }

      // Unfortunately, really slow path.

      bool disconnected = false;

      if (nsContentUtils::ComparePoints(range->StartRef().AsRaw(),

                                        firstRange->StartRef().AsRaw(),

                                        &disconnected) == -1) {

        firstRange = range;

      }

    }

  }

  return firstRange ? firstRange->StartRef().AsRaw() : RawRangeBoundary();

}



RawRangeBoundary TextComposition::GetEndRef() const {

  RefPtr<EditorBase> editorBase = GetEditorBase();

  if (!editorBase) {

    return RawRangeBoundary();

  }



  nsISelectionController* selectionController =

      editorBase->GetSelectionController();

  if (NS_WARN_IF(!selectionController)) {

    return RawRangeBoundary();

  }



  nsRange* lastRange = nullptr;

  static const SelectionType kIMESelectionTypes[] = {

      SelectionType::eIMERawClause, SelectionType::eIMESelectedRawClause,

      SelectionType::eIMEConvertedClause, SelectionType::eIMESelectedClause};

  for (auto selectionType : kIMESelectionTypes) {

    Selection* selection =

        selectionController->GetSelection(ToRawSelectionType(selectionType));

    if (!selection) {

      continue;

    }

    for (uint32_t i = 0; i < selection->RangeCount(); i++) {

      nsRange* range = selection->GetRangeAt(i);

      if (NS_WARN_IF(!range) || NS_WARN_IF(!range->GetEndContainer())) {

        continue;

      }

      if (!lastRange) {

        lastRange = range;

        continue;

      }

      // In most cases, all composition string should be in same text node.

      if (lastRange->GetEndContainer() == range->GetEndContainer()) {

        if (lastRange->EndOffset() < range->EndOffset()) {

          lastRange = range;

        }

        continue;

      }

      // However, if web apps have inserted different nodes in composition

      // string, composition string may span 2 or more nodes.

      if (lastRange->GetEndContainer() ==

          range->GetEndContainer()->GetNextSibling()) {

        // Fast path for some known applications like Google Keep.

        lastRange = range;

        continue;

      }

      // Unfortunately, really slow path.

      bool disconnected = false;

      if (nsContentUtils::ComparePoints(lastRange->EndRef().AsRaw(),

                                        range->EndRef().AsRaw(),

                                        &disconnected) == -1) {

        lastRange = range;

      }

    }

  }

  return lastRange ? lastRange->EndRef().AsRaw() : RawRangeBoundary();

}



/******************************************************************************

 * TextComposition::CompositionEventDispatcher

 ******************************************************************************/

dom/events/TextComposition.h

+11 −0
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ 
#include "nsPresContext.h"

#include "mozilla/Attributes.h"

#include "mozilla/EventForwards.h"

#include "mozilla/RangeBoundary.h"

#include "mozilla/TextRange.h"

#include "mozilla/dom/TabParent.h"

#include "mozilla/dom/Text.h"
@@ -125,6 +126,16 @@ class TextComposition final { 
    return mCompositionStartOffset + mTargetClauseOffsetInComposition;

  }



  /**

   * Return current composition start and end point in the DOM tree.

   * Note that one of or both of those result container may be different

   * from GetContainerTextNode() if the DOM tree was modified by the web

   * app.  If there is no composition string the DOM tree, these return

   * unset range boundaries.

   */

  RawRangeBoundary GetStartRef() const;

  RawRangeBoundary GetEndRef() const;



  /**

   * The offset of composition string in the text node.  If composition string

   * hasn't been inserted in any text node yet, this returns UINT32_MAX.

editor/libeditor/EditorBase.cpp

+10 −37
Original line number Diff line number Diff line
@@ -3895,6 +3895,16 @@ void EditorBase::EndUpdateViewBatch() { 


TextComposition* EditorBase::GetComposition() const { return mComposition; }



EditorRawDOMPoint EditorBase::GetCompositionStartPoint() const {

  return mComposition ? EditorRawDOMPoint(mComposition->GetStartRef())

                      : EditorRawDOMPoint();

}



EditorRawDOMPoint EditorBase::GetCompositionEndPoint() const {

  return mComposition ? EditorRawDOMPoint(mComposition->GetEndRef())

                      : EditorRawDOMPoint();

}



bool EditorBase::IsIMEComposing() const {

  return mComposition && mComposition->IsComposing();

}
@@ -4805,43 +4815,6 @@ void EditorBase::OnFocus(EventTarget* aFocusEventTarget) { 
  }

}



int32_t EditorBase::GetIMESelectionStartOffsetIn(nsINode* aTextNode) {

  MOZ_ASSERT(aTextNode, "aTextNode must not be nullptr");



  nsISelectionController* selectionController = GetSelectionController();

  if (NS_WARN_IF(!selectionController)) {

    return -1;

  }



  uint32_t minOffset = UINT32_MAX;

  static const SelectionType kIMESelectionTypes[] = {

      SelectionType::eIMERawClause, SelectionType::eIMESelectedRawClause,

      SelectionType::eIMEConvertedClause, SelectionType::eIMESelectedClause};

  for (auto selectionType : kIMESelectionTypes) {

    RefPtr<Selection> selection = GetSelection(selectionType);

    if (!selection) {

      continue;

    }

    for (uint32_t i = 0; i < selection->RangeCount(); i++) {

      RefPtr<nsRange> range = selection->GetRangeAt(i);

      if (NS_WARN_IF(!range)) {

        continue;

      }

      if (NS_WARN_IF(range->GetStartContainer() != aTextNode)) {

        // ignore the start offset...

      } else {

        minOffset = std::min(minOffset, range->StartOffset());

      }

      if (NS_WARN_IF(range->GetEndContainer() != aTextNode)) {

        // ignore the end offset...

      } else {

        minOffset = std::min(minOffset, range->EndOffset());

      }

    }

  }

  return minOffset < INT32_MAX ? minOffset : -1;

}



void EditorBase::HideCaret(bool aHide) {

  if (mHidingCaret == aHide) {

    return;

editor/libeditor/EditorBase.h

+8 −6
Original line number Diff line number Diff line
@@ -866,6 +866,14 @@ class EditorBase : public nsIEditor, 
    return mEditActionData->RangeUpdaterRef();

  }



  /**

   * GetCompositionStartPoint() and GetCompositionEndPoint() returns start and

   * end point of composition string if there is.  Otherwise, returns non-set

   * DOM point.

   */

  EditorRawDOMPoint GetCompositionStartPoint() const;

  EditorRawDOMPoint GetCompositionEndPoint() const;



  /**

   * InsertTextWithTransaction() inserts aStringToInsert to aPointToInsert or

   * better insertion point around it.  If aPointToInsert isn't in a text node,
@@ -1713,12 +1721,6 @@ class EditorBase : public nsIEditor, 
   */

  virtual bool IsActiveInDOMWindow();



  /**

   * GetIMESelectionStartOffsetIn() returns the start offset of IME selection in

   * the aTextNode.  If there is no IME selection, returns -1.

   */

  int32_t GetIMESelectionStartOffsetIn(nsINode* aTextNode);



  /**

   * FindBetterInsertionPoint() tries to look for better insertion point which

   * is typically the nearest text node and offset in it.

editor/libeditor/HTMLEditRules.cpp

+63 −43
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ 
#include "mozilla/MathAlgorithms.h"

#include "mozilla/Move.h"

#include "mozilla/Preferences.h"

#include "mozilla/TextComposition.h"

#include "mozilla/UniquePtr.h"

#include "mozilla/Unused.h"

#include "mozilla/dom/Selection.h"
@@ -337,12 +338,17 @@ nsresult HTMLEditRules::BeforeEdit(EditSubAction aEditSubAction, 
    AutoSafeEditorData setData(*this, *mHTMLEditor);



    // Remember where our selection was before edit action took place:



    if (HTMLEditorRef().GetCompositionStartPoint().IsSet()) {

      // If there is composition string, let's remember current composition

      // range.

      mRangeItem->StoreRange(HTMLEditorRef().GetCompositionStartPoint(), HTMLEditorRef().GetCompositionEndPoint());

    } else {

      // Get the selection location

      if (!SelectionRefPtr()->RangeCount()) {

        return NS_ERROR_UNEXPECTED;

      }

      mRangeItem->StoreRange(SelectionRefPtr()->GetRangeAt(0));

    }

    nsCOMPtr<nsINode> selStartNode = mRangeItem->mStartContainer;

    nsCOMPtr<nsINode> selEndNode = mRangeItem->mEndContainer;
@@ -529,7 +535,24 @@ nsresult HTMLEditRules::AfterEditInner(EditSubAction aEditSubAction, 
        aEditSubAction == EditSubAction::eInsertParagraphSeparator ||

        aEditSubAction == EditSubAction::ePasteHTMLContent ||

        aEditSubAction == EditSubAction::eInsertHTMLSource) {

      rv = AdjustWhitespace();

      // TODO: Temporarily, WSRunObject replaces ASCII whitespaces with NPSPs

      //       and then, we'll replace them with ASCII whitespaces here.  We

      //       should avoid this overwriting things as far as possible because

      //       replacing characters in text nodes causes running mutation event

      //       listeners which are really expensive.

      // Adjust end of composition string if there is composition string.

      EditorRawDOMPoint pointToAdjust(HTMLEditorRef().GetCompositionEndPoint());

      if (!pointToAdjust.IsSet()) {

        // Otherwise, adjust current selection start point.

        pointToAdjust = EditorBase::GetStartPoint(*SelectionRefPtr());

        if (NS_WARN_IF(!pointToAdjust.IsSet())) {

          return NS_ERROR_FAILURE;

        }

      }

      rv = WSRunObject(&HTMLEditorRef(), pointToAdjust).AdjustWhitespace();

      if (NS_WARN_IF(!CanHandleEditAction())) {

        return NS_ERROR_EDITOR_DESTROYED;

      }

      if (NS_WARN_IF(NS_FAILED(rv))) {

        return rv;

      }
@@ -1331,21 +1354,19 @@ nsresult HTMLEditRules::WillInsertText(EditSubAction aEditSubAction, 
  }



  if (aEditSubAction == EditSubAction::eInsertTextComingFromIME) {

    // Right now the WSRunObject code bails on empty strings, but IME needs

    // the InsertTextWithTransaction() call to still happen since empty strings

    // are meaningful there.

    // If there is one or more IME selections, its minimum offset should be

    // the insertion point.

    int32_t IMESelectionOffset = HTMLEditorRef().GetIMESelectionStartOffsetIn(

        pointToInsert.GetContainer());

    if (IMESelectionOffset >= 0) {

      pointToInsert.Set(pointToInsert.GetContainer(), IMESelectionOffset);

    EditorRawDOMPoint compositionStartPoint =

        HTMLEditorRef().GetCompositionStartPoint();

    if (!compositionStartPoint.IsSet()) {

      compositionStartPoint = pointToInsert;

    }



    if (inString->IsEmpty()) {

      // Right now the WSRunObject code bails on empty strings, but IME needs

      // the InsertTextWithTransaction() call to still happen since empty

      // strings are meaningful there.

      rv = MOZ_KnownLive(HTMLEditorRef())

               .InsertTextWithTransaction(*doc, *inString,

                                          EditorRawDOMPoint(pointToInsert));

                                          compositionStartPoint);

      if (NS_WARN_IF(!CanHandleEditAction())) {

        return NS_ERROR_EDITOR_DESTROYED;

      }
@@ -1355,14 +1376,36 @@ nsresult HTMLEditRules::WillInsertText(EditSubAction aEditSubAction, 
      return NS_OK;

    }



    WSRunObject wsObj(&HTMLEditorRef(), pointToInsert);

    rv = wsObj.InsertText(*doc, *inString, pointToInsert);

    EditorRawDOMPoint compositionEndPoint =

        HTMLEditorRef().GetCompositionEndPoint();

    if (!compositionEndPoint.IsSet()) {

      compositionEndPoint = compositionStartPoint;

    }

    WSRunObject wsObj(&HTMLEditorRef(), compositionStartPoint,

                      compositionEndPoint);

    rv = wsObj.InsertText(*doc, *inString);

    if (NS_WARN_IF(!CanHandleEditAction())) {

      return NS_ERROR_EDITOR_DESTROYED;

    }

    if (NS_WARN_IF(NS_FAILED(rv))) {

      return rv;

    }



    compositionStartPoint = HTMLEditorRef().GetCompositionStartPoint();

    compositionEndPoint = HTMLEditorRef().GetCompositionEndPoint();

    if (NS_WARN_IF(!compositionStartPoint.IsSet()) ||

        NS_WARN_IF(!compositionEndPoint.IsSet())) {

      // Mutation event listener has changed the DOM tree...

      return NS_OK;

    }

    if (!mDocChangeRange) {

      mDocChangeRange = new nsRange(compositionStartPoint.GetContainer());

    }

    rv = mDocChangeRange->SetStartAndEnd(compositionStartPoint,

                                         compositionEndPoint);

    if (NS_WARN_IF(NS_FAILED(rv))) {

      return rv;

    }

    return NS_OK;

  }
@@ -1486,8 +1529,7 @@ nsresult HTMLEditRules::WillInsertText(EditSubAction aEditSubAction, 
        // is it a tab?

        if (subStr.Equals(tabStr)) {

          EditorRawDOMPoint pointAfterInsertedSpaces;

          rv = wsObj.InsertText(*doc, spacesStr, currentPoint,

                                &pointAfterInsertedSpaces);

          rv = wsObj.InsertText(*doc, spacesStr, &pointAfterInsertedSpaces);

          if (NS_WARN_IF(!CanHandleEditAction())) {

            return NS_ERROR_EDITOR_DESTROYED;

          }
@@ -1527,8 +1569,7 @@ nsresult HTMLEditRules::WillInsertText(EditSubAction aEditSubAction, 
              "Perhaps, newBRElement has been moved or removed unexpectedly");

        } else {

          EditorRawDOMPoint pointAfterInsertedString;

          rv = wsObj.InsertText(*doc, subStr, currentPoint,

                                &pointAfterInsertedString);

          rv = wsObj.InsertText(*doc, subStr, &pointAfterInsertedString);

          if (NS_WARN_IF(!CanHandleEditAction())) {

            return NS_ERROR_EDITOR_DESTROYED;

          }
@@ -9054,27 +9095,6 @@ HTMLEditRules::InsertBRElementToEmptyListItemsAndTableCellsInChangedRange() { 
  return NS_OK;

}



nsresult HTMLEditRules::AdjustWhitespace() {

  MOZ_ASSERT(IsEditorDataAvailable());



  EditorRawDOMPoint selectionStartPoint(

      EditorBase::GetStartPoint(*SelectionRefPtr()));

  if (NS_WARN_IF(!selectionStartPoint.IsSet())) {

    return NS_ERROR_FAILURE;

  }



  // Ask whitespace object to tweak nbsp's

  nsresult rv =

      WSRunObject(&HTMLEditorRef(), selectionStartPoint).AdjustWhitespace();

  if (NS_WARN_IF(!CanHandleEditAction())) {

    return NS_ERROR_EDITOR_DESTROYED;

  }

  if (NS_WARN_IF(NS_FAILED(rv))) {

    return rv;

  }

  return NS_OK;

}



nsresult HTMLEditRules::PinSelectionToNewBlock() {

  MOZ_ASSERT(IsEditorDataAvailable());