Bug 1375825 - part2: ContentEventHandler::ExpandToClusterBoundary() should check the return value of nsTextFrame::PeekOffsetCharacter() r=jfkthame

ContentEventHandler::ExpandToClusterBoundary() doesn't check the return value of nsTextFrame::PeekOffsetCharacter().  Therefore, it may set its result to reversed offset. (e.g., when aForward is true and offset is 6, the result may be 5.  When aForward is false and offset is 5, the result may be 6.)

For avoiding that, ContentEventHandler::ExpandToClusterBoundary() should check the result and only when it returns nsIFrame::FOUND, it should compute the proper offset.

On the other hand, it's too bad for ContentEventHandler that nsTextFrame::PeekOffsetCharacter() to return nsIFrame::CONTINUE_UNSELECTABLE when the user-select style is "all" because IME doesn't expect such cases.

Therefore, this patch adds additional argument to nsIFrame::PeekOffsetCharacter(), aOptions which is a struct containing bool members.  The reason why it's not a bit mask enum is, such struct doesn't cause simple mistake at checking the value and the code is shorter.  When mIgnoreUserStyleAll of it is true, this patch makes nsTextFrame not return nsIFrame::CONTINUE_UNSELECTABLE.

MozReview-Commit-ID: ACNNBTP92YZ

--HG--
extra : rebase_source : bd85da902e7fb59135d15514cb20a5599a4a640b
This commit is contained in:
Masayuki Nakano 2017-06-29 10:58:16 +09:00
Родитель 179e1ee17e
Коммит 5a78a77b68
12 изменённых файлов: 111 добавлений и 57 удалений

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

@ -939,31 +939,41 @@ ContentEventHandler::ExpandToClusterBoundary(nsIContent* aContent,
aForward ? CARET_ASSOCIATE_BEFORE : CARET_ASSOCIATE_AFTER;
nsIFrame* frame = fs->GetFrameForNodeOffset(aContent, int32_t(*aXPOffset),
hint, &offsetInFrame);
if (!frame) {
// This content doesn't have any frames, we only can check surrogate pair...
const nsTextFragment* text = aContent->GetText();
NS_ENSURE_TRUE(text, NS_ERROR_FAILURE);
if (NS_IS_LOW_SURROGATE(text->CharAt(*aXPOffset)) &&
NS_IS_HIGH_SURROGATE(text->CharAt(*aXPOffset - 1))) {
*aXPOffset += aForward ? 1 : -1;
if (frame) {
int32_t startOffset, endOffset;
nsresult rv = frame->GetOffsets(startOffset, endOffset);
NS_ENSURE_SUCCESS(rv, rv);
if (*aXPOffset == static_cast<uint32_t>(startOffset) ||
*aXPOffset == static_cast<uint32_t>(endOffset)) {
return NS_OK;
}
if (!frame->IsTextFrame()) {
return NS_ERROR_FAILURE;
}
nsTextFrame* textFrame = static_cast<nsTextFrame*>(frame);
int32_t newOffsetInFrame = *aXPOffset - startOffset;
newOffsetInFrame += aForward ? -1 : 1;
// PeekOffsetCharacter() should respect cluster but ignore user-select
// style. If it returns "FOUND", we should use the result. Otherwise,
// we shouldn't use the result because the offset was moved to reversed
// direction.
nsTextFrame::PeekOffsetCharacterOptions options;
options.mRespectClusters = true;
options.mIgnoreUserStyleAll = true;
if (textFrame->PeekOffsetCharacter(aForward, &newOffsetInFrame,
options) == nsIFrame::FOUND) {
*aXPOffset = startOffset + newOffsetInFrame;
return NS_OK;
}
return NS_OK;
}
int32_t startOffset, endOffset;
nsresult rv = frame->GetOffsets(startOffset, endOffset);
NS_ENSURE_SUCCESS(rv, rv);
if (*aXPOffset == static_cast<uint32_t>(startOffset) ||
*aXPOffset == static_cast<uint32_t>(endOffset)) {
return NS_OK;
// If the frame isn't available, we only can check surrogate pair...
const nsTextFragment* text = aContent->GetText();
NS_ENSURE_TRUE(text, NS_ERROR_FAILURE);
if (NS_IS_LOW_SURROGATE(text->CharAt(*aXPOffset)) &&
NS_IS_HIGH_SURROGATE(text->CharAt(*aXPOffset - 1))) {
*aXPOffset += aForward ? 1 : -1;
}
if (!frame->IsTextFrame()) {
return NS_ERROR_FAILURE;
}
nsTextFrame* textFrame = static_cast<nsTextFrame*>(frame);
int32_t newOffsetInFrame = *aXPOffset - startOffset;
newOffsetInFrame += aForward ? -1 : 1;
textFrame->PeekOffsetCharacter(aForward, &newOffsetInFrame);
*aXPOffset = startOffset + newOffsetInFrame;
return NS_OK;
}

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

@ -34,8 +34,10 @@ public:
virtual ContentOffsets CalcContentOffsetsFromFramePoint(nsPoint aPoint) override;
virtual FrameSearchResult PeekOffsetNoAmount(bool aForward, int32_t* aOffset) override;
virtual FrameSearchResult PeekOffsetCharacter(bool aForward, int32_t* aOffset,
bool aRespectClusters = true) override;
virtual FrameSearchResult
PeekOffsetCharacter(bool aForward, int32_t* aOffset,
PeekOffsetCharacterOptions aOptions =
PeekOffsetCharacterOptions()) override;
virtual FrameSearchResult PeekOffsetWord(bool aForward, bool aWordSelectEatSpace,
bool aIsKeyboardSelect, int32_t* aOffset,
PeekWordState* aState) override;
@ -244,7 +246,7 @@ BRFrame::PeekOffsetNoAmount(bool aForward, int32_t* aOffset)
nsIFrame::FrameSearchResult
BRFrame::PeekOffsetCharacter(bool aForward, int32_t* aOffset,
bool aRespectClusters)
PeekOffsetCharacterOptions aOptions)
{
NS_ASSERTION (aOffset && *aOffset <= 1, "aOffset out of range");
// Keep going. The actual line jumping will stop us.

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

@ -28,10 +28,12 @@ public:
{
return nsFrame::PeekOffsetNoAmount(aForward, aOffset);
}
FrameSearchResult PeekOffsetCharacter(bool aForward, int32_t* aOffset,
bool aRespectClusters = true) override
FrameSearchResult
PeekOffsetCharacter(bool aForward, int32_t* aOffset,
PeekOffsetCharacterOptions aOptions =
PeekOffsetCharacterOptions()) override
{
return nsFrame::PeekOffsetCharacter(aForward, aOffset, aRespectClusters);
return nsFrame::PeekOffsetCharacter(aForward, aOffset, aOptions);
}
nsSplittableType GetSplittableType() const override
{

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

@ -402,8 +402,9 @@ nsContainerFrame::PeekOffsetNoAmount(bool aForward, int32_t* aOffset)
}
nsIFrame::FrameSearchResult
nsContainerFrame::PeekOffsetCharacter(bool aForward, int32_t* aOffset,
bool aRespectClusters)
nsContainerFrame::PeekOffsetCharacter(
bool aForward, int32_t* aOffset,
PeekOffsetCharacterOptions aOptions)
{
NS_ASSERTION (aOffset && *aOffset <= 1, "aOffset out of range");
// Don't allow the caret to stay in an empty (leaf) container frame.

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

@ -63,8 +63,10 @@ public:
virtual void ChildIsDirty(nsIFrame* aChild) override;
virtual FrameSearchResult PeekOffsetNoAmount(bool aForward, int32_t* aOffset) override;
virtual FrameSearchResult PeekOffsetCharacter(bool aForward, int32_t* aOffset,
bool aRespectClusters = true) override;
virtual FrameSearchResult
PeekOffsetCharacter(bool aForward, int32_t* aOffset,
PeekOffsetCharacterOptions aOptions =
PeekOffsetCharacterOptions()) override;
virtual nsresult AttributeChanged(int32_t aNameSpaceID,
nsIAtom* aAttribute,

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

@ -8025,11 +8025,14 @@ nsIFrame::PeekOffset(nsPeekOffsetStruct* aPos)
bool movingInFrameDirection =
IsMovingInFrameDirection(current, aPos->mDirection, aPos->mVisual);
if (eatingNonRenderableWS)
if (eatingNonRenderableWS) {
peekSearchState = current->PeekOffsetNoAmount(movingInFrameDirection, &offset);
else
peekSearchState = current->PeekOffsetCharacter(movingInFrameDirection, &offset,
aPos->mAmount == eSelectCluster);
} else {
PeekOffsetCharacterOptions options;
options.mRespectClusters = aPos->mAmount == eSelectCluster;
peekSearchState = current->PeekOffsetCharacter(movingInFrameDirection,
&offset, options);
}
movedOverNonSelectableText |= (peekSearchState == CONTINUE_UNSELECTABLE);
@ -8363,7 +8366,7 @@ nsFrame::PeekOffsetNoAmount(bool aForward, int32_t* aOffset)
nsIFrame::FrameSearchResult
nsFrame::PeekOffsetCharacter(bool aForward, int32_t* aOffset,
bool aRespectClusters)
PeekOffsetCharacterOptions aOptions)
{
NS_ASSERTION (aOffset && *aOffset <= 1, "aOffset out of range");
int32_t startOffset = *aOffset;

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

@ -211,8 +211,10 @@ public:
FrameSearchResult PeekOffsetNoAmount(bool aForward,
int32_t* aOffset) override;
FrameSearchResult PeekOffsetCharacter(bool aForward, int32_t* aOffset,
bool aRespectClusters = true) override;
FrameSearchResult
PeekOffsetCharacter(bool aForward, int32_t* aOffset,
PeekOffsetCharacterOptions aOptions =
PeekOffsetCharacterOptions()) override;
FrameSearchResult PeekOffsetWord(bool aForward, bool aWordSelectEatSpace,
bool aIsKeyboardSelect,
int32_t* aOffset,

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

@ -667,6 +667,26 @@ public:
CONTINUE_UNSELECTABLE = 0x4 | CONTINUE,
};
/**
* Options for PeekOffsetCharacter().
*/
struct MOZ_STACK_CLASS PeekOffsetCharacterOptions
{
// Whether to restrict result to valid cursor locations (between grapheme
// clusters) - if this is included, maintains "normal" behavior, otherwise,
// used for selection by "code unit" (instead of "character")
bool mRespectClusters;
// Whether to check user-select style value - if this is included, checks
// if user-select is all, then, it may return CONTINUE_UNSELECTABLE.
bool mIgnoreUserStyleAll;
PeekOffsetCharacterOptions()
: mRespectClusters(true)
, mIgnoreUserStyleAll(false)
{
}
};
protected:
/**
* Return true if the frame is part of a Selection.
@ -4117,16 +4137,19 @@ protected:
* @param aForward [in] Are we moving forward (or backward) in content order.
* @param aOffset [in/out] At what offset into the frame to start looking.
* on output - what offset was reached (whether or not we found a place to stop).
* @param aRespectClusters [in] Whether to restrict result to valid cursor locations
* (between grapheme clusters) - default TRUE maintains "normal" behavior,
* FALSE is used for selection by "code unit" (instead of "character")
* @param aOptions [in] Options, see the comment in
* PeekOffsetCharacterOptions for the detail.
* @return STOP: An appropriate offset was found within this frame,
* and is given by aOffset.
* CONTINUE: Not found within this frame, need to try the next frame.
* see enum FrameSearchResult for more details.
*/
virtual FrameSearchResult PeekOffsetCharacter(bool aForward, int32_t* aOffset,
bool aRespectClusters = true) = 0;
virtual FrameSearchResult
PeekOffsetCharacter(bool aForward, int32_t* aOffset,
PeekOffsetCharacterOptions aOptions =
PeekOffsetCharacterOptions()) = 0;
static_assert(sizeof(PeekOffsetCharacterOptions) <= sizeof(intptr_t),
"aOptions should be changed to const reference");
/**
* Search the frame for the next word boundary

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

@ -170,7 +170,7 @@ nsInlineFrame::IsEmpty()
nsIFrame::FrameSearchResult
nsInlineFrame::PeekOffsetCharacter(bool aForward, int32_t* aOffset,
bool aRespectClusters)
PeekOffsetCharacterOptions aOptions)
{
// Override the implementation in nsFrame, to skip empty inline frames
NS_ASSERTION (aOffset && *aOffset <= 1, "aOffset out of range");

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

@ -56,8 +56,10 @@ public:
virtual bool IsEmpty() override;
virtual bool IsSelfEmpty() override;
virtual FrameSearchResult PeekOffsetCharacter(bool aForward, int32_t* aOffset,
bool aRespectClusters = true) override;
virtual FrameSearchResult
PeekOffsetCharacter(bool aForward, int32_t* aOffset,
PeekOffsetCharacterOptions aOptions =
PeekOffsetCharacterOptions()) override;
virtual void DestroyFrom(nsIFrame* aDestructRoot) override;
virtual nsresult StealFrame(nsIFrame* aChild) override;

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

@ -7984,15 +7984,18 @@ IsAcceptableCaretPosition(const gfxSkipCharsIterator& aIter,
nsIFrame::FrameSearchResult
nsTextFrame::PeekOffsetCharacter(bool aForward, int32_t* aOffset,
bool aRespectClusters)
PeekOffsetCharacterOptions aOptions)
{
int32_t contentLength = GetContentLength();
NS_ASSERTION(aOffset && *aOffset <= contentLength, "aOffset out of range");
StyleUserSelect selectStyle;
IsSelectable(&selectStyle);
if (selectStyle == StyleUserSelect::All)
return CONTINUE_UNSELECTABLE;
if (!aOptions.mIgnoreUserStyleAll) {
StyleUserSelect selectStyle;
IsSelectable(&selectStyle);
if (selectStyle == StyleUserSelect::All) {
return CONTINUE_UNSELECTABLE;
}
}
gfxSkipCharsIterator iter = EnsureTextRun(nsTextFrame::eInflated);
if (!mTextRun)
@ -8008,7 +8011,8 @@ nsTextFrame::PeekOffsetCharacter(bool aForward, int32_t* aOffset,
for (int32_t i = std::min(trimmed.GetEnd(), startOffset) - 1;
i >= trimmed.mStart; --i) {
iter.SetOriginalOffset(i);
if (IsAcceptableCaretPosition(iter, aRespectClusters, mTextRun, this)) {
if (IsAcceptableCaretPosition(iter, aOptions.mRespectClusters, mTextRun,
this)) {
*aOffset = i - mContentOffset;
return FOUND;
}
@ -8025,7 +8029,8 @@ nsTextFrame::PeekOffsetCharacter(bool aForward, int32_t* aOffset,
for (int32_t i = startOffset + 1; i <= trimmed.GetEnd(); ++i) {
iter.SetOriginalOffset(i);
if (i == trimmed.GetEnd() ||
IsAcceptableCaretPosition(iter, aRespectClusters, mTextRun, this)) {
IsAcceptableCaretPosition(iter, aOptions.mRespectClusters, mTextRun,
this)) {
*aOffset = i - mContentOffset;
return FOUND;
}

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

@ -188,9 +188,11 @@ public:
FrameSearchResult PeekOffsetNoAmount(bool aForward,
int32_t* aOffset) override;
FrameSearchResult PeekOffsetCharacter(bool aForward,
int32_t* aOffset,
bool aRespectClusters = true) override;
FrameSearchResult
PeekOffsetCharacter(bool aForward,
int32_t* aOffset,
PeekOffsetCharacterOptions aOptions =
PeekOffsetCharacterOptions()) override;
FrameSearchResult PeekOffsetWord(bool aForward,
bool aWordSelectEatSpace,
bool aIsKeyboardSelect,