Skip to content

Commit

Permalink
Bug 1726297 - part 2: Make `ContentEventHandler::Get(First|Last)Frame…
Browse files Browse the repository at this point in the history
…InRangeForTextRect` ignore invisible nodes r=smaug

The callers of them want to return text rect for first visible thing in the
given range to make IME place UI around there.  Therefore, it should ignore
invisible text which won't be interactive with IME.

Differential Revision: https://phabricator.services.mozilla.com/D160591
  • Loading branch information
masayuki-nakano committed Nov 11, 2022
1 parent 327a2b8 commit 15a9cf9
Showing 1 changed file with 38 additions and 17 deletions.
55 changes: 38 additions & 17 deletions dom/events/ContentEventHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1505,27 +1505,36 @@ ContentEventHandler::GetFirstFrameInRangeForTextRect(
break;
}

if (!node->IsContent()) {
auto* content = nsIContent::FromNode(node);
if (MOZ_UNLIKELY(!content)) {
continue;
}

if (node->IsText()) {
// If the node is invisible (e.g., the node is or is in an invisible node or
// it's a white-space only text node around a block boundary), we should
// ignore it.
if (!content->GetPrimaryFrame()) {
continue;
}

if (auto* textNode = Text::FromNode(content)) {
// If the range starts at the end of a text node, we need to find
// next node which causes text.
const uint32_t offsetInNode =
node == aRawRange.GetStartContainer() ? aRawRange.StartOffset() : 0u;
if (offsetInNode < node->Length()) {
nodePosition = {node, offsetInNode};
const uint32_t offsetInNode = textNode == aRawRange.GetStartContainer()
? aRawRange.StartOffset()
: 0u;
if (offsetInNode < textNode->TextDataLength()) {
nodePosition = {textNode, offsetInNode};
break;
}
continue;
}

// If the element node causes a line break before it, it's the first
// node causing text.
if (ShouldBreakLineBefore(*node->AsContent(), mRootElement) ||
IsPaddingBR(*node->AsContent())) {
nodePosition = {node, 0u};
if (ShouldBreakLineBefore(*content, mRootElement) ||
IsPaddingBR(*content)) {
nodePosition = {content, 0u};
}
}

Expand Down Expand Up @@ -1592,14 +1601,26 @@ ContentEventHandler::GetLastFrameInRangeForTextRect(const RawRange& aRawRange) {
break;
}

if (!node->IsContent() || node == nextNodeOfRangeEnd) {
if (node == nextNodeOfRangeEnd) {
continue;
}

auto* content = nsIContent::FromNode(node);
if (MOZ_UNLIKELY(!content)) {
continue;
}

// If the node is invisible (e.g., the node is or is in an invisible node or
// it's a white-space only text node around a block boundary), we should
// ignore it.
if (!content->GetPrimaryFrame()) {
continue;
}

if (node->IsText()) {
nodePosition = {node, node == aRawRange.GetEndContainer()
? aRawRange.EndOffset()
: node->Length()};
if (auto* textNode = Text::FromNode(node)) {
nodePosition = {textNode, textNode == aRawRange.GetEndContainer()
? aRawRange.EndOffset()
: textNode->TextDataLength()};

// If the text node is empty or the last node of the range but the index
// is 0, we should store current position but continue looking for
Expand All @@ -1612,9 +1633,9 @@ ContentEventHandler::GetLastFrameInRangeForTextRect(const RawRange& aRawRange) {
break;
}

if (ShouldBreakLineBefore(*node->AsContent(), mRootElement) ||
IsPaddingBR(*node->AsContent())) {
nodePosition = {node, 0u};
if (ShouldBreakLineBefore(*content, mRootElement) ||
IsPaddingBR(*content)) {
nodePosition = {content, 0u};
break;
}
}
Expand Down

0 comments on commit 15a9cf9

Please sign in to comment.