Skip to content

Commit

Permalink
Bug 1286464 part.18 ContentEventHandler::OnQueryTextRectArray() shoul…
Browse files Browse the repository at this point in the history
…d compute line breaker's rect with the last character's node when the start of queried range is a line breaker and it's caused by a block frame r=smaug

Similar to OnQueryTextRect(), OnQueryTextRectArray() should compute a line breaker's rect with the last character when it's the first character in queried range and not caused by <br> frame.

MozReview-Commit-ID: CGDHbcrc6zB
  • Loading branch information
masayuki-nakano committed Aug 8, 2016
1 parent 58c4072 commit ea6f234
Show file tree
Hide file tree
Showing 2 changed files with 146 additions and 60 deletions.
197 changes: 137 additions & 60 deletions dom/events/ContentEventHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1688,6 +1688,40 @@ ContentEventHandler::GetLineBreakerRectBefore(nsIFrame* aFrame)
return result;
}

ContentEventHandler::FrameRelativeRect
ContentEventHandler::GuessLineBreakerRectAfter(nsIContent* aTextContent)
{
// aTextContent should be a text node.
MOZ_ASSERT(aTextContent->IsNodeOfType(nsINode::eTEXT));

FrameRelativeRect result;
int32_t length = static_cast<int32_t>(aTextContent->Length());
if (NS_WARN_IF(length < 0)) {
return result;
}
// Get the last nsTextFrame which is caused by aTextContent. Note that
// a text node can cause multiple text frames, e.g., the text is too long
// and wrapped by its parent block or the text has line breakers and its
// white-space property respects the line breakers (e.g., |pre|).
nsIFrame* lastTextFrame = nullptr;
nsresult rv = GetFrameForTextRect(aTextContent, length, true, &lastTextFrame);
if (NS_WARN_IF(NS_FAILED(rv)) || NS_WARN_IF(!lastTextFrame)) {
return result;
}
const nsRect kLastTextFrameRect = lastTextFrame->GetRect();
if (lastTextFrame->GetWritingMode().IsVertical()) {
// Below of the last text frame.
result.mRect.SetRect(0, kLastTextFrameRect.height,
kLastTextFrameRect.width, 0);
} else {
// Right of the last text frame (not bidi-aware).
result.mRect.SetRect(kLastTextFrameRect.width, 0,
0, kLastTextFrameRect.height);
}
result.mBaseFrame = lastTextFrame;
return result;
}

nsresult
ContentEventHandler::OnQueryTextRectArray(WidgetQueryContentEvent* aEvent)
{
Expand All @@ -1708,8 +1742,9 @@ ContentEventHandler::OnQueryTextRectArray(WidgetQueryContentEvent* aEvent)
bool wasLineBreaker = false;
nsRect lastCharRect;
while (offset < kEndOffset) {
nsCOMPtr<nsIContent> lastTextContent;
rv = SetRangeFromFlatTextOffset(range, offset, 1, lineBreakType, true,
nullptr);
nullptr, getter_AddRefs(lastTextContent));
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
Expand Down Expand Up @@ -1749,19 +1784,76 @@ ContentEventHandler::OnQueryTextRectArray(WidgetQueryContentEvent* aEvent)

AutoTArray<nsRect, 16> charRects;

if (ShouldBreakLineBefore(firstContent, mRootContent) ||
IsMozBR(firstContent)) {
// If the first frame is a text frame, the result should be computed with
// the frame's API.
if (firstFrame->GetType() == nsGkAtoms::textFrame) {
rv = firstFrame->GetCharacterRectsInRange(firstFrame.mOffsetInNode,
kEndOffset - offset, charRects);
if (NS_WARN_IF(NS_FAILED(rv)) || NS_WARN_IF(charRects.IsEmpty())) {
return rv;
}
// Assign the characters whose rects are computed by the call of
// nsTextFrame::GetCharacterRectsInRange().
AppendSubString(chars, firstContent, firstFrame.mOffsetInNode,
charRects.Length());
if (NS_WARN_IF(chars.Length() != charRects.Length())) {
return NS_ERROR_UNEXPECTED;
}
if (kBRLength > 1 && chars[0] == '\n' &&
offset == aEvent->mInput.mOffset && offset) {
// If start of range starting from previous offset of query range is
// same as the start of query range, the query range starts from
// between a line breaker (i.e., the range starts between "\r" and
// "\n").
RefPtr<nsRange> rangeToPrevOffset = new nsRange(mRootContent);
rv = SetRangeFromFlatTextOffset(rangeToPrevOffset,
aEvent->mInput.mOffset - 1, 1,
lineBreakType, true, nullptr);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
startsBetweenLineBreaker =
range->GetStartParent() == rangeToPrevOffset->GetStartParent() &&
range->StartOffset() == rangeToPrevOffset->StartOffset();
}
}
// Other contents should cause a line breaker rect before it.
// Note that moz-<br> element does not cause any text, however,
// it represents empty line at the last of current block. Therefore,
// we need to compute its rect too.
else if (ShouldBreakLineBefore(firstContent, mRootContent) ||
IsMozBR(firstContent)) {
nsRect brRect;
// If the frame is <br> frame, we can always trust
// GetLineBreakerRectBefore(). Otherwise, "after" the last character
// rect is better than its result.
// TODO: We need to look for the last character rect if non-br frame
// causes a line break at first time of this loop.
if (firstFrame->GetType() == nsGkAtoms::brFrame ||
aEvent->mInput.mOffset == offset) {
FrameRelativeRect relativeBRRect = GetLineBreakerRectBefore(firstFrame);
brRect = relativeBRRect.RectRelativeTo(firstFrame);
} else {
// If the frame is not a <br> frame, we need to compute the caret rect
// with last character's rect before firstContent if there is.
// For example, if caret is after "c" of |<p>abc</p><p>def</p>|, IME may
// query a line breaker's rect after "c". Then, if we compute it only
// with the 2nd <p>'s block frame, the result will be:
// +-<p>--------------------------------+
// |abc |
// +------------------------------------+
//
// I+-<p>--------------------------------+
// |def |
// +------------------------------------+
// However, users expect popup windows of IME should be positioned at
// right-bottom of "c" like this:
// +-<p>--------------------------------+
// |abcI |
// +------------------------------------+
//
// +-<p>--------------------------------+
// |def |
// +------------------------------------+
// Therefore, if the first frame isn't a <br> frame and there is a text
// node before the first node in the queried range, we should compute the
// first rect with the previous character's rect.
// If we already compute a character's rect in the queried range, we can
// compute it with the cached last character's rect. (However, don't
// use this path if it's a <br> frame because trusting <br> frame's rect
// is better than guessing the rect from the previous character.)
if (firstFrame->GetType() != nsGkAtoms::brFrame &&
aEvent->mInput.mOffset != offset) {
// The frame position in the root widget will be added in the
// following for loop but we need the rect in the previous frame.
// So, we need to avoid using current frame position.
Expand All @@ -1778,6 +1870,30 @@ ContentEventHandler::OnQueryTextRectArray(WidgetQueryContentEvent* aEvent)
}
}
}
// If it's not a <br> frame and it's the first character rect at the
// queried range, we need to the previous character of the start of
// the queried range if there is a text node.
else if (firstFrame->GetType() != nsGkAtoms::brFrame && lastTextContent) {
FrameRelativeRect brRectRelativeToLastTextFrame =
GuessLineBreakerRectAfter(lastTextContent);
if (NS_WARN_IF(!brRectRelativeToLastTextFrame.IsValid())) {
return NS_ERROR_FAILURE;
}
brRect = brRectRelativeToLastTextFrame.RectRelativeTo(firstFrame);
}
// Otherwise, we need to compute the line breaker's rect only with the
// first frame's rect. But this may be unexpected. For example,
// |<div contenteditable>[<p>]abc</p></div>|. In this case, caret is
// before "a", therefore, users expect the rect left of "a". However,
// we don't have enough information about the next character here and
// this isn't usual case (e.g., IME typically tries to query the rect
// of "a" or caret rect for computing its popup position). Therefore,
// we shouldn't do more complicated hack here unless we'll get some bug
// reports actually.
else {
FrameRelativeRect relativeBRRect = GetLineBreakerRectBefore(firstFrame);
brRect = relativeBRRect.RectRelativeTo(firstFrame);
}
charRects.AppendElement(brRect);
chars.AssignLiteral("\n");
if (kBRLength > 1 && offset == aEvent->mInput.mOffset && offset) {
Expand All @@ -1795,35 +1911,9 @@ ContentEventHandler::OnQueryTextRectArray(WidgetQueryContentEvent* aEvent)
startsBetweenLineBreaker = frameForPrevious.mFrame == firstFrame.mFrame;
}
} else {
rv = firstFrame->GetCharacterRectsInRange(firstFrame.mOffsetInNode,
kEndOffset - offset, charRects);
if (NS_WARN_IF(NS_FAILED(rv)) || NS_WARN_IF(charRects.IsEmpty())) {
return rv;
}
// Assign the characters whose rects are computed by the call of
// nsTextFrame::GetCharacterRectsInRange().
AppendSubString(chars, firstContent, firstFrame.mOffsetInNode,
charRects.Length());
if (NS_WARN_IF(chars.Length() != charRects.Length())) {
return NS_ERROR_UNEXPECTED;
}
if (kBRLength > 1 && chars[0] == '\n' &&
offset == aEvent->mInput.mOffset && offset) {
// If start of range starting from previous offset of query range is
// same as the start of query range, the query range starts from
// between a line breaker (i.e., the range starts between "\r" and
// "\n").
RefPtr<nsRange> rangeToPrevOffset = new nsRange(mRootContent);
rv = SetRangeFromFlatTextOffset(rangeToPrevOffset,
aEvent->mInput.mOffset - 1, 1,
lineBreakType, true, nullptr);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
startsBetweenLineBreaker =
range->GetStartParent() == rangeToPrevOffset->GetStartParent() &&
range->StartOffset() == rangeToPrevOffset->StartOffset();
}
NS_WARNING("The frame is neither a text frame nor a frame whose content "
"causes a line break");
return NS_ERROR_FAILURE;
}

for (size_t i = 0; i < charRects.Length() && offset < kEndOffset; i++) {
Expand Down Expand Up @@ -2012,26 +2102,13 @@ ContentEventHandler::OnQueryTextRect(WidgetQueryContentEvent* aEvent)
// node before the first node in the queried range, we should compute the
// first rect with the previous character's rect.
else if (firstFrame->GetType() != nsGkAtoms::brFrame && lastTextContent) {
int32_t length = static_cast<int32_t>(lastTextContent->Length());
if (NS_WARN_IF(length < 0)) {
return NS_ERROR_FAILURE;
}
nsIFrame* lastTextFrame = nullptr;
rv = GetFrameForTextRect(lastTextContent, length, true, &lastTextFrame);
if (NS_WARN_IF(NS_FAILED(rv)) || NS_WARN_IF(!lastTextFrame)) {
FrameRelativeRect brRectAfterLastChar =
GuessLineBreakerRectAfter(lastTextContent);
if (NS_WARN_IF(!brRectAfterLastChar.IsValid())) {
return NS_ERROR_FAILURE;
}
const nsRect kLastTextFrameRect = lastTextFrame->GetRect();
if (lastTextFrame->GetWritingMode().IsVertical()) {
// Below of the last text frame.
rect.SetRect(0, kLastTextFrameRect.height,
kLastTextFrameRect.width, 0);
} else {
// Right of the last text frame (not bidi-aware).
rect.SetRect(kLastTextFrameRect.width, 0,
0, kLastTextFrameRect.height);
}
rv = ConvertToRootRelativeOffset(lastTextFrame, rect);
rect = brRectAfterLastChar.mRect;
rv = ConvertToRootRelativeOffset(brRectAfterLastChar.mBaseFrame, rect);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
Expand Down
9 changes: 9 additions & 0 deletions dom/events/ContentEventHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,15 @@ class MOZ_STACK_CLASS ContentEventHandler
// doesn't check if aFrame should cause line break in non-debug build.
FrameRelativeRect GetLineBreakerRectBefore(nsIFrame* aFrame);

// Returns a line breaker rect after aTextContent as there is a line breaker
// immediately after aTextContent. This is useful when following block
// element causes a line break before it and it needs to compute the line
// breaker's rect. For example, if there is |<p>abc</p><p>def</p>|, the
// rect of 2nd <p>'s line breaker should be at right of "c" in the first
// <p>, not the start of 2nd <p>. The result is relative to the last text
// frame which represents the last character of aTextContent.
FrameRelativeRect GuessLineBreakerRectAfter(nsIContent* aTextContent);

// Make aRect non-empty. If width and/or height is 0, these methods set them
// to 1. Note that it doesn't set nsRect's width nor height to one device
// pixel because using nsRect::ToOutsidePixels() makes actual width or height
Expand Down

0 comments on commit ea6f234

Please sign in to comment.