Bug 1647556 - part 9-2: Make `WSRunObject::InsertBreak()` stop using `FindNearestFragment()` r=m_kato

It removes some invisible leading and/or trailing white-spaces when it inserts
`<br>` element into the invisible white-space sequence.  It currently checks
whether the insertion point is in invisible leading and trailing white-spaces
or not with `FindNearestFragment()`, but we can do same thing with
comparing the insertion point with the result of
`TextFragmentData::GetInvisibleLeadingWhiteSpaceRange()` and
`TextFragmentData::GetInvisibleLeadingWhiteSpaceRange()`.  However, current
implementation does not make sense because:
- It checks trailing white-spaces with `!IsEndOfHardLine()` and
  `IsStartOfHardLine()`, but this means that it does ignores invisible
   white-spaces which are the only content in a line.
- It checks leading white-spaces with `!IsStartOfHardLine()` and
  `IsEndOfHardLine()`, so, this also ignores invisible white-spaces which
  are the only content in a line.
- The important thing of the logic is prevent that invisible leading and
  trailing white-spaces become visible with new `<br>` element, but this
  is done only for trailing white-spaces.

Differential Revision: https://phabricator.services.mozilla.com/D82283
This commit is contained in:
Masayuki Nakano 2020-07-09 09:44:41 +00:00
Родитель d806bac0e0
Коммит dd8f12dbc3
3 изменённых файлов: 188 добавлений и 60 удалений

Просмотреть файл

@ -1014,6 +1014,11 @@ class EditorDOMRangeBase final {
return mStart == mEnd;
}
bool IsPositioned() const { return mStart.IsSet() && mEnd.IsSet(); }
template <typename OtherPointType>
bool Contains(const OtherPointType& aPoint) const {
return IsPositioned() && mStart.EqualsOrIsBefore(aPoint) &&
mEnd.IsBefore(aPoint);
}
bool InSameContainer() const {
MOZ_ASSERT(IsPositioned());
return IsPositioned() && mStart.GetContainer() == mEnd.GetContainer();

Просмотреть файл

@ -202,12 +202,18 @@ already_AddRefed<Element> WSRunObject::InsertBreak(
// meanwhile, the pre case is handled in HandleInsertText() in
// HTMLEditSubActionHandler.cpp
const WSFragment* beforeRun = FindNearestFragment(aPointToInsert, false);
const WSFragment* afterRun = FindNearestFragment(aPointToInsert, true);
TextFragmentData textFragmentData(mStart, mEnd, mNBSPData, mPRE);
const EditorDOMRange invisibleLeadingWhiteSpaceRangeOfNewLine =
textFragmentData.GetNewInvisibleLeadingWhiteSpaceRangeIfSplittingAt(
aPointToInsert);
const EditorDOMRange invisibleTrailingWhiteSpaceRangeOfCurrentLine =
textFragmentData.GetNewInvisibleTrailingWhiteSpaceRangeIfSplittingAt(
aPointToInsert);
const Maybe<const WSFragment> visibleWSFragmentInMiddleOfLine =
TextFragmentData(mStart, mEnd, mNBSPData, mPRE)
.CreateWSFragmentForVisibleAndMiddleOfLine();
!invisibleLeadingWhiteSpaceRangeOfNewLine.IsPositioned() ||
!invisibleTrailingWhiteSpaceRangeOfCurrentLine.IsPositioned()
? textFragmentData.CreateWSFragmentForVisibleAndMiddleOfLine()
: Nothing();
const PointPosition pointPositionWithVisibleWhiteSpaces =
visibleWSFragmentInMiddleOfLine.isSome()
? visibleWSFragmentInMiddleOfLine.ref().ComparePoint(aPointToInsert)
@ -219,22 +225,23 @@ already_AddRefed<Element> WSRunObject::InsertBreak(
// point while we tweak any surrounding white-space
AutoTrackDOMPoint tracker(mHTMLEditor.RangeUpdaterRef(), &pointToInsert);
// Handle any changes needed to ws run after inserted br
if (!afterRun || afterRun->IsEndOfHardLine()) {
// Don't need to do anything. Just insert break. ws won't change.
} else if (afterRun->IsStartOfHardLine()) {
// Delete the leading ws that is after insertion point. We don't
// have to (it would still not be significant after br), but it's
// just more aesthetically pleasing to.
nsresult rv = MOZ_KnownLive(mHTMLEditor)
if (invisibleTrailingWhiteSpaceRangeOfCurrentLine.IsPositioned()) {
if (!invisibleTrailingWhiteSpaceRangeOfCurrentLine.Collapsed()) {
// XXX Why don't we remove all of the invisible white-spaces?
MOZ_ASSERT(invisibleTrailingWhiteSpaceRangeOfCurrentLine.StartRef() ==
pointToInsert);
nsresult rv =
MOZ_KnownLive(mHTMLEditor)
.DeleteTextAndTextNodesWithTransaction(
pointToInsert, afterRun->EndPoint());
invisibleTrailingWhiteSpaceRangeOfCurrentLine.StartRef(),
invisibleTrailingWhiteSpaceRangeOfCurrentLine.EndRef());
if (NS_FAILED(rv)) {
NS_WARNING(
"HTMLEditor::DeleteTextAndTextNodesWithTransaction() failed");
return nullptr;
}
}
}
// If new line will start with visible white-spaces, it needs to be start
// with an NBSP.
else if (pointPositionWithVisibleWhiteSpaces ==
@ -265,21 +272,26 @@ already_AddRefed<Element> WSRunObject::InsertBreak(
}
}
// Handle any changes needed to ws run before inserted br
if (!beforeRun || beforeRun->IsStartOfHardLine()) {
// Don't need to do anything. Just insert break. ws won't change.
} else if (beforeRun->IsEndOfHardLine()) {
// Need to delete the trailing ws that is before insertion point, because
// it would become significant after break inserted.
nsresult rv = MOZ_KnownLive(mHTMLEditor)
if (invisibleLeadingWhiteSpaceRangeOfNewLine.IsPositioned()) {
if (!invisibleLeadingWhiteSpaceRangeOfNewLine.Collapsed()) {
// XXX Why don't we remove all of the invisible white-spaces?
MOZ_ASSERT(invisibleLeadingWhiteSpaceRangeOfNewLine.EndRef() ==
pointToInsert);
// XXX If the DOM tree has been changed above,
// invisibleLeadingWhiteSpaceRangeOfNewLine may be invalid now.
// So, we may do something wrong here.
nsresult rv =
MOZ_KnownLive(mHTMLEditor)
.DeleteTextAndTextNodesWithTransaction(
beforeRun->StartPoint(), pointToInsert);
invisibleLeadingWhiteSpaceRangeOfNewLine.StartRef(),
invisibleLeadingWhiteSpaceRangeOfNewLine.EndRef());
if (NS_FAILED(rv)) {
NS_WARNING(
"WSRunObject::DeleteTextAndTextNodesWithTransaction() failed");
return nullptr;
}
}
}
// If the `<br>` element is put immediately after an NBSP, it should be
// replaced with an ASCII white-space.
else if (pointPositionWithVisibleWhiteSpaces ==
@ -982,41 +994,51 @@ void WSRunScanner::EnsureWSFragments() {
textFragmentData.InitializeWSFragmentArray(mFragments);
}
template <typename EditorDOMRangeType>
EditorDOMRangeType
EditorDOMRange
WSRunScanner::TextFragmentData::GetInvisibleLeadingWhiteSpaceRange() const {
if (mLeadingWhiteSpaceRange.isSome()) {
return mLeadingWhiteSpaceRange.ref();
}
// If it's preformatted or not start of line, the range is not invisible
// leading white-spaces.
// TODO: We should check each text node style rather than WSRunScanner's
// scan start position's style.
if (mIsPreformatted || !StartsFromHardLineBreak()) {
return EditorDOMRangeType();
mLeadingWhiteSpaceRange.emplace();
return mLeadingWhiteSpaceRange.ref();
}
// If there is no NBSP, all of the given range is leading white-spaces.
// Note that this result may be collapsed if there is no leading white-spaces.
if (!mNBSPData.FoundNBSP()) {
MOZ_ASSERT(mStart.PointRef().IsSet() || mEnd.PointRef().IsSet());
return EditorDOMRangeType(mStart.PointRef(), mEnd.PointRef());
mLeadingWhiteSpaceRange.emplace(mStart.PointRef(), mEnd.PointRef());
return mLeadingWhiteSpaceRange.ref();
}
MOZ_ASSERT(mNBSPData.LastPointRef().IsSetAndValid());
// Even if the first NBSP is the start, i.e., there is no invisible leading
// white-space, return collapsed range.
return EditorDOMRangeType(mStart.PointRef(), mNBSPData.FirstPointRef());
mLeadingWhiteSpaceRange.emplace(mStart.PointRef(), mNBSPData.FirstPointRef());
return mLeadingWhiteSpaceRange.ref();
}
template <typename EditorDOMRangeType>
EditorDOMRangeType
EditorDOMRange
WSRunScanner::TextFragmentData::GetInvisibleTrailingWhiteSpaceRange() const {
if (mTrailingWhiteSpaceRange.isSome()) {
return mTrailingWhiteSpaceRange.ref();
}
// If it's preformatted or not immediately before block boundary, the range is
// not invisible trailing white-spaces. Note that collapsible white-spaces
// before a `<br>` element is visible.
// TODO: We should check each text node style rather than WSRunScanner's
// scan start position's style.
if (mIsPreformatted || !EndsByBlockBoundary()) {
return EditorDOMRangeType();
mTrailingWhiteSpaceRange.emplace();
return mTrailingWhiteSpaceRange.ref();
}
// If there is no NBSP, all of the given range is trailing white-spaces.
@ -1024,7 +1046,8 @@ WSRunScanner::TextFragmentData::GetInvisibleTrailingWhiteSpaceRange() const {
// spaces.
if (!mNBSPData.FoundNBSP()) {
MOZ_ASSERT(mStart.PointRef().IsSet() || mEnd.PointRef().IsSet());
return EditorDOMRangeType(mStart.PointRef(), mEnd.PointRef());
mTrailingWhiteSpaceRange.emplace(mStart.PointRef(), mEnd.PointRef());
return mTrailingWhiteSpaceRange.ref();
}
MOZ_ASSERT(mNBSPData.LastPointRef().IsSetAndValid());
@ -1035,13 +1058,15 @@ WSRunScanner::TextFragmentData::GetInvisibleTrailingWhiteSpaceRange() const {
mNBSPData.LastPointRef().GetContainer() ==
mEnd.PointRef().GetContainer() &&
mNBSPData.LastPointRef().Offset() == mEnd.PointRef().Offset() - 1) {
return EditorDOMRangeType();
mTrailingWhiteSpaceRange.emplace();
return mTrailingWhiteSpaceRange.ref();
}
// Otherwise, the may be some trailing white-spaces.
MOZ_ASSERT(!mNBSPData.LastPointRef().IsEndOfContainer());
return EditorDOMRangeType(mNBSPData.LastPointRef().NextPoint(),
mTrailingWhiteSpaceRange.emplace(mNBSPData.LastPointRef().NextPoint(),
mEnd.PointRef());
return mTrailingWhiteSpaceRange.ref();
}
Maybe<WSRunScanner::WSFragment>
@ -1067,8 +1092,8 @@ WSRunScanner::TextFragmentData::CreateWSFragmentForVisibleAndMiddleOfLine()
// If all of the range is invisible leading or trailing white-spaces,
// there is no visible content.
const auto leadingWhiteSpaceRange =
GetInvisibleLeadingWhiteSpaceRange<EditorRawDOMRange>();
const EditorDOMRange leadingWhiteSpaceRange =
GetInvisibleLeadingWhiteSpaceRange();
const bool maybeHaveLeadingWhiteSpaces =
leadingWhiteSpaceRange.StartRef().IsSet() ||
leadingWhiteSpaceRange.EndRef().IsSet();
@ -1077,8 +1102,8 @@ WSRunScanner::TextFragmentData::CreateWSFragmentForVisibleAndMiddleOfLine()
leadingWhiteSpaceRange.EndRef() == mEnd.PointRef()) {
return Nothing();
}
const auto trailingWhiteSpaceRange =
GetInvisibleTrailingWhiteSpaceRange<EditorRawDOMRange>();
const EditorDOMRange trailingWhiteSpaceRange =
GetInvisibleTrailingWhiteSpaceRange();
const bool maybeHaveTrailingWhiteSpaces =
trailingWhiteSpaceRange.StartRef().IsSet() ||
trailingWhiteSpaceRange.EndRef().IsSet();
@ -1168,10 +1193,10 @@ void WSRunScanner::TextFragmentData::InitializeWSFragmentArray(
return;
}
const auto leadingWhiteSpaceRange =
GetInvisibleLeadingWhiteSpaceRange<EditorRawDOMRange>();
const auto trailingWhiteSpaceRange =
GetInvisibleTrailingWhiteSpaceRange<EditorRawDOMRange>();
const EditorDOMRange leadingWhiteSpaceRange =
GetInvisibleLeadingWhiteSpaceRange();
const EditorDOMRange trailingWhiteSpaceRange =
GetInvisibleTrailingWhiteSpaceRange();
// XXX `IsPositioned()` is not availble here because currently it is not
// guaranteed that both boundaries are set or unset at same time.
const bool maybeHaveLeadingWhiteSpaces =
@ -2142,12 +2167,12 @@ nsresult WSRunObject::MaybeReplaceInclusiveNextNBSPWithASCIIWhiteSpace(
nsresult WSRunObject::DeleteInvisibleASCIIWhiteSpacesInternal() {
TextFragmentData textFragment(mStart, mEnd, mNBSPData, mPRE);
auto leadingWhiteSpaceRange =
textFragment.GetInvisibleLeadingWhiteSpaceRange<EditorDOMRange>();
EditorDOMRange leadingWhiteSpaceRange =
textFragment.GetInvisibleLeadingWhiteSpaceRange();
// XXX Getting trailing white-space range now must be wrong because
// mutation event listener may invalidate it.
auto trailingWhiteSpaceRange =
textFragment.GetInvisibleTrailingWhiteSpaceRange<EditorDOMRange>();
EditorDOMRange trailingWhiteSpaceRange =
textFragment.GetInvisibleTrailingWhiteSpaceRange();
DebugOnly<bool> leadingWhiteSpacesDeleted = false;
if (leadingWhiteSpaceRange.IsPositioned() &&
!leadingWhiteSpaceRange.Collapsed()) {

Просмотреть файл

@ -774,8 +774,7 @@ class MOZ_STACK_CLASS WSRunScanner {
* Note that if there are only invisible white-spaces in a hard line,
* this returns all of the white-spaces.
*/
template <typename EditorDOMRangeType>
EditorDOMRangeType GetInvisibleLeadingWhiteSpaceRange() const;
EditorDOMRange GetInvisibleLeadingWhiteSpaceRange() const;
/**
* GetInvisibleTrailingWhiteSpaceRange() returns two DOM points,
@ -785,8 +784,105 @@ class MOZ_STACK_CLASS WSRunScanner {
* Note that if there are only invisible white-spaces in a hard line,
* this returns all of the white-spaces.
*/
template <typename EditorDOMRangeType>
EditorDOMRangeType GetInvisibleTrailingWhiteSpaceRange() const;
EditorDOMRange GetInvisibleTrailingWhiteSpaceRange() const;
/**
* GetNewInvisibleLeadingWhiteSpaceRangeIfSplittingAt() returns new
* invisible leading white-space range which should be removed if
* splitting invisible white-space sequence at aPointToSplit creates
* new invisible leading white-spaces in the new line.
* Note that the result may be collapsed range if the point is around
* invisible white-spaces.
*/
template <typename EditorDOMPointType>
EditorDOMRange GetNewInvisibleLeadingWhiteSpaceRangeIfSplittingAt(
const EditorDOMPointType& aPointToSplit) const {
// If there are invisible trailing white-spaces and some or all of them
// become invisible leading white-spaces in the new line, although we
// don't need to delete them, but for aesthetically and backward
// compatibility, we should remove them.
EditorDOMRange trailingWhiteSpaceRange =
GetInvisibleTrailingWhiteSpaceRange();
// XXX Why don't we check leading white-spaces too?
if (!trailingWhiteSpaceRange.IsPositioned()) {
return trailingWhiteSpaceRange;
}
// XXX Why don't we need to treat new trailing white-spaces are invisible
// when the trailing white-spaces are only the content in current
// line?
if (trailingWhiteSpaceRange != GetInvisibleLeadingWhiteSpaceRange()) {
return EditorDOMRange();
}
// If the point is before the trailing white-spaces, the new line won't
// start with leading white-spaces.
if (aPointToSplit.IsBefore(trailingWhiteSpaceRange.StartRef())) {
return EditorDOMRange();
}
// If the point is in the trailing white-spaces, the new line may
// start with some leading white-spaces. Returning collapsed range
// is intentional because the caller may want to know whether the
// point is in trailing white-spaces or not.
if (aPointToSplit.EqualsOrIsBefore(trailingWhiteSpaceRange.EndRef())) {
return EditorDOMRange(trailingWhiteSpaceRange.StartRef(),
aPointToSplit);
}
// Otherwise, if the point is after the trailing white-spaces, it may
// be just outside of the text node. E.g., end of parent element.
// This is possible case but the validation cost is not worthwhile
// due to the runtime cost in the worst case. Therefore, we should just
// return collapsed range at the end of trailing white-spaces. Then,
// callers can know the point is immediately after the trailing
// white-spaces.
return EditorDOMRange(trailingWhiteSpaceRange.EndRef());
}
/**
* GetNewInvisibleTrailingWhiteSpaceRangeIfSplittingAt() returns new
* invisible trailing white-space range which should be removed if
* splitting invisible white-space sequence at aPointToSplit creates
* new invisible trailing white-spaces in the new line.
* Note that the result may be collapsed range if the point is around
* invisible white-spaces.
*/
template <typename EditorDOMPointType>
EditorDOMRange GetNewInvisibleTrailingWhiteSpaceRangeIfSplittingAt(
const EditorDOMPointType& aPointToSplit) const {
// If there are invisible leading white-spaces and some or all of them
// become end of current line, they will become visible. Therefore, we
// need to delete the invisible leading white-spaces before insertion
// point.
EditorDOMRange leadingWhiteSpaceRange =
GetInvisibleLeadingWhiteSpaceRange();
if (!leadingWhiteSpaceRange.IsPositioned()) {
return leadingWhiteSpaceRange;
}
// XXX Why don't we need to treat new leading white-spaces are invisible
// when the leading white-spaces are only the content in current
// line?
if (leadingWhiteSpaceRange != GetInvisibleTrailingWhiteSpaceRange()) {
return EditorDOMRange();
}
// If the point equals or is after the leading white-spaces, the line
// will end without trailing white-spaces.
if (leadingWhiteSpaceRange.EndRef().IsBefore(aPointToSplit)) {
return EditorDOMRange();
}
// If the point is in the leading white-spaces, the line may
// end with some trailing white-spaces. Returning collapsed range
// is intentional because the caller may want to know whether the
// point is in leading white-spaces or not.
if (leadingWhiteSpaceRange.StartRef().EqualsOrIsBefore(aPointToSplit)) {
return EditorDOMRange(aPointToSplit, leadingWhiteSpaceRange.EndRef());
}
// Otherwise, if the point is before the leading white-spaces, it may
// be just outside of the text node. E.g., start of parent element.
// This is possible case but the validation cost is not worthwhile
// due to the runtime cost in the worst case. Therefore, we should
// just return collapsed range at start of the leading white-spaces.
// Then, callers can know the point is immediately before the leading
// white-spaces.
return EditorDOMRange(leadingWhiteSpaceRange.StartRef());
}
/**
* CreateWSFragmentForVisibleAndMiddleOfLine() creates WSFragment whose
@ -798,6 +894,8 @@ class MOZ_STACK_CLASS WSRunScanner {
BoundaryData mStart;
BoundaryData mEnd;
NoBreakingSpaceData mNBSPData;
mutable Maybe<EditorDOMRange> mLeadingWhiteSpaceRange;
mutable Maybe<EditorDOMRange> mTrailingWhiteSpaceRange;
// XXX Currently we set mIsPreformatted to `WSRunScanner::mPRE` value
// even if some text nodes between mStart and mEnd are different styled
// nodes. This caused some bugs actually, but we now keep traditional