Diff
Modified: trunk/Source/WebCore/ChangeLog (259929 => 259930)
--- trunk/Source/WebCore/ChangeLog 2020-04-11 15:14:34 UTC (rev 259929)
+++ trunk/Source/WebCore/ChangeLog 2020-04-11 15:35:04 UTC (rev 259930)
@@ -1,3 +1,80 @@
+2020-04-08 Darin Adler <[email protected]>
+
+ Use Node::length to replace Node::maxCharacterOffset and lastOffsetInNode; switch more offsets from int to unsigned
+ https://bugs.webkit.org/show_bug.cgi?id=210246
+
+ Reviewed by Antti Koivisto.
+
+ - The recently-added Node::length, which matches the DOM specification terminology
+ for node offsets as used in ranges, is the same as the existing maxCharacterOffset
+ and lastOffsetInNode functions. Deleted all uses of those and replaced them
+ with calls to Node::length. One of the benefits of this is that Node::length is
+ implemented more efficiently and is not a virtual function. Another is consistently
+ matching the DOM specification terminology.
+ - Many offsets, including the ones in live ranges, are currently implemented as signed
+ in WebKit, but are specified as unsigned in the DOM and HTML specifications. This
+ has very little observable effect from _javascript_ that can affect website compatibility,
+ but it's still helpful to be consistent both with the specification and internally.
+ Accordingly, changed some of these to unsigned; more to come later.
+
+ * accessibility/AXObjectCache.cpp:
+ (WebCore::AXObjectCache::previousBoundary): Use length instead of
+ maxCharacterOffset.
+
+ * dom/CharacterData.cpp:
+ (WebCore::CharacterData::maxCharacterOffset const): Deleted.
+ * dom/CharacterData.h: Deleted maxCharacterOffset override.
+
+ * dom/DocumentMarkerController.cpp:
+ (WebCore::DocumentMarkerController::shiftMarkers): Use length instead of
+ maxCharacterOffset.
+
+ * dom/Node.cpp:
+ (WebCore::Node::maxCharacterOffset const): Deleted.
+ * dom/Node.h: Deleted maxCharacterOffset.
+
+ * dom/Position.cpp:
+ (WebCore::Position::computeOffsetInContainerNode const): Use length instead
+ of lastOffsetInNode.
+
+ * dom/Position.h:
+ (WebCore::lastOffsetInNode): Deleted.
+ (WebCore::lastPositionInNode): Use length instead of lastOffsetInNode.
+ (WebCore::minOffsetForNode): Use length instead of maxCharacterOffset.
+ (WebCore::offsetIsBeforeLastNodeOffset): Ditto.
+
+ * dom/RangeBoundaryPoint.h:
+ (WebCore::RangeBoundaryPoint::setToEndOfNode): Use length instead of
+ maxCharacterOffset.
+
+ * editing/ApplyBlockElementCommand.cpp:
+ (WebCore::isNewLineAtPosition): Use length instead of maxCharacterOffset.
+ (WebCore::ApplyBlockElementCommand::rangeForParagraphSplittingTextNodesIfNeeded):
+ Ditto.
+
+ * editing/ApplyStyleCommand.cpp:
+ (WebCore::ApplyStyleCommand::removeInlineStyle): Use length instead of
+ maxCharacterOffset.
+
+ * editing/Editing.cpp:
+ (WebCore::lastOffsetForEditing): Use length instead of mmaxCharacterOffset
+ and countChildNodes. Also improved the comment here.
+
+ * editing/EditingStyle.cpp:
+ (WebCore::EditingStyle::styleAtSelectionStart): Use length instead of
+ maxCharacterOffset.
+
+ * editing/InsertListCommand.cpp:
+ (WebCore::InsertListCommand::doApplyForSingleParagraph): Use length instead
+ of lastOffsetInNode.
+
+ * editing/TextIterator.cpp:
+ (WebCore::SimplifiedBackwardsTextIterator::SimplifiedBackwardsTextIterator):
+ USe length instead of lastOffsetInNode.
+
+ * editing/VisibleUnits.cpp:
+ (WebCore::previousBoundary): Use length instead of maxCharacterOffset.
+
2020-04-10 Alex Christensen <[email protected]>
PersistentCoders should use modern decoding syntax
Modified: trunk/Source/WebCore/accessibility/AXObjectCache.cpp (259929 => 259930)
--- trunk/Source/WebCore/accessibility/AXObjectCache.cpp 2020-04-11 15:14:34 UTC (rev 259929)
+++ trunk/Source/WebCore/accessibility/AXObjectCache.cpp 2020-04-11 15:35:04 UTC (rev 259930)
@@ -2664,7 +2664,7 @@
if (&node != characterOffset.node && AccessibilityObject::replacedNodeNeedsCharacter(nextSibling))
return startOrEndCharacterOffsetForRange(rangeForNodeContents(nextSibling), false);
- if ((!suffixLength && node.isTextNode() && static_cast<int>(next) <= node.maxCharacterOffset()) || (node.renderer() && node.renderer()->isBR() && !next)) {
+ if ((!suffixLength && node.isTextNode() && next <= node.length()) || (node.renderer() && node.renderer()->isBR() && !next)) {
// The next variable contains a usable index into a text node
if (node.isTextNode())
return traverseToOffsetInRange(rangeForNodeContents(&node), next, TraverseOptionValidateOffset);
Modified: trunk/Source/WebCore/accessibility/atk/WebKitAccessibleUtil.cpp (259929 => 259930)
--- trunk/Source/WebCore/accessibility/atk/WebKitAccessibleUtil.cpp 2020-04-11 15:14:34 UTC (rev 259929)
+++ trunk/Source/WebCore/accessibility/atk/WebKitAccessibleUtil.cpp 2020-04-11 15:35:04 UTC (rev 259930)
@@ -168,7 +168,7 @@
// node is actually inside the region, at least partially.
auto& node = *coreObject->node();
auto* lastDescendant = node.lastDescendant();
- unsigned lastOffset = lastOffsetInNode(lastDescendant);
+ unsigned lastOffset = lastDescendant->length();
auto intersectsResult = range->intersectsNode(node);
return !intersectsResult.hasException()
&& intersectsResult.releaseReturnValue()
Modified: trunk/Source/WebCore/dom/CharacterData.cpp (259929 => 259930)
--- trunk/Source/WebCore/dom/CharacterData.cpp 2020-04-11 15:14:34 UTC (rev 259929)
+++ trunk/Source/WebCore/dom/CharacterData.cpp 2020-04-11 15:35:04 UTC (rev 259930)
@@ -239,9 +239,4 @@
InspectorInstrumentation::characterDataModified(document(), *this);
}
-int CharacterData::maxCharacterOffset() const
-{
- return static_cast<int>(length());
-}
-
} // namespace WebCore
Modified: trunk/Source/WebCore/dom/CharacterData.h (259929 => 259930)
--- trunk/Source/WebCore/dom/CharacterData.h 2020-04-11 15:14:34 UTC (rev 259929)
+++ trunk/Source/WebCore/dom/CharacterData.h 2020-04-11 15:35:04 UTC (rev 259930)
@@ -63,7 +63,6 @@
String nodeValue() const final;
ExceptionOr<void> setNodeValue(const String&) final;
bool virtualIsCharacterData() const final { return true; }
- int maxCharacterOffset() const final;
void setDataAndUpdate(const String&, unsigned offsetOfReplacedData, unsigned oldLength, unsigned newLength);
void notifyParentAfterChange(ContainerNode::ChildChangeSource);
Modified: trunk/Source/WebCore/dom/DocumentMarkerController.cpp (259929 => 259930)
--- trunk/Source/WebCore/dom/DocumentMarkerController.cpp 2020-04-11 15:14:34 UTC (rev 259929)
+++ trunk/Source/WebCore/dom/DocumentMarkerController.cpp 2020-04-11 15:35:04 UTC (rev 259930)
@@ -597,7 +597,7 @@
// FIXME: No obvious reason this should be iOS-specific. Remove the #if at some point.
int targetStartOffset = marker.startOffset() + delta;
int targetEndOffset = marker.endOffset() + delta;
- if (targetStartOffset >= node.maxCharacterOffset() || targetEndOffset <= 0) {
+ if (static_cast<unsigned>(targetStartOffset) >= node.length() || targetEndOffset <= 0) {
list->remove(i);
continue;
}
@@ -615,7 +615,7 @@
list->remove(i);
continue;
}
- marker.setEndOffset(targetEndOffset < node.maxCharacterOffset() ? targetEndOffset : node.maxCharacterOffset());
+ marker.setEndOffset(std::min<unsigned>(targetEndOffset, node.length()));
didShiftMarker = true;
}
#endif
Modified: trunk/Source/WebCore/dom/Node.cpp (259929 => 259930)
--- trunk/Source/WebCore/dom/Node.cpp 2020-04-11 15:14:34 UTC (rev 259929)
+++ trunk/Source/WebCore/dom/Node.cpp 2020-04-11 15:35:04 UTC (rev 259930)
@@ -1102,12 +1102,6 @@
return composedParent->computedStyle(pseudoElementSpecifier);
}
-int Node::maxCharacterOffset() const
-{
- ASSERT_NOT_REACHED();
- return 0;
-}
-
// FIXME: Shouldn't these functions be in the editing code? Code that asks questions about HTML in the core DOM class
// is obviously misplaced.
bool Node::canStartSelection() const
Modified: trunk/Source/WebCore/dom/Node.h (259929 => 259930)
--- trunk/Source/WebCore/dom/Node.h 2020-04-11 15:14:34 UTC (rev 259929)
+++ trunk/Source/WebCore/dom/Node.h 2020-04-11 15:35:04 UTC (rev 259930)
@@ -377,10 +377,6 @@
WEBCORE_EXPORT bool contains(const Node*) const;
bool containsIncludingShadowDOM(const Node*) const;
- // Number of DOM 16-bit units contained in node. Note that rendered text length can be different - e.g. because of
- // css-transform:capitalize breaking up precomposed characters and ligatures.
- virtual int maxCharacterOffset() const;
-
// Whether or not a selection can be started in this object
virtual bool canStartSelection() const;
Modified: trunk/Source/WebCore/dom/Position.cpp (259929 => 259930)
--- trunk/Source/WebCore/dom/Position.cpp 2020-04-11 15:14:34 UTC (rev 259929)
+++ trunk/Source/WebCore/dom/Position.cpp 2020-04-11 15:35:04 UTC (rev 259930)
@@ -207,7 +207,7 @@
case PositionIsBeforeChildren:
return 0;
case PositionIsAfterChildren:
- return lastOffsetInNode(m_anchorNode.get());
+ return m_anchorNode->length();
case PositionIsOffsetInAnchor:
return minOffsetForNode(m_anchorNode.get(), m_offset);
case PositionIsBeforeAnchor:
Modified: trunk/Source/WebCore/dom/Position.h (259929 => 259930)
--- trunk/Source/WebCore/dom/Position.h 2020-04-11 15:14:34 UTC (rev 259929)
+++ trunk/Source/WebCore/dom/Position.h 2020-04-11 15:35:04 UTC (rev 259930)
@@ -25,6 +25,7 @@
#pragma once
+#include "CharacterData.h"
#include "ContainerNode.h"
#include "EditingBoundary.h"
#include "TextAffinity.h"
@@ -237,8 +238,6 @@
Position positionBeforeNode(Node* anchorNode);
Position positionAfterNode(Node* anchorNode);
-int lastOffsetInNode(Node*);
-
// firstPositionInNode and lastPositionInNode return parent-anchored positions, lastPositionInNode construction is O(n) due to countChildNodes()
Position firstPositionInNode(Node* anchorNode);
Position lastPositionInNode(Node* anchorNode);
@@ -306,11 +305,6 @@
return Position(anchorNode, Position::PositionIsAfterAnchor);
}
-inline int lastOffsetInNode(Node* node)
-{
- return node->isCharacterDataNode() ? node->maxCharacterOffset() : node->countChildNodes();
-}
-
// firstPositionInNode and lastPositionInNode return parent-anchored positions, lastPositionInNode construction is O(n) due to countChildNodes()
inline Position firstPositionInNode(Node* anchorNode)
{
@@ -322,32 +316,29 @@
inline Position lastPositionInNode(Node* anchorNode)
{
if (anchorNode->isTextNode())
- return Position(anchorNode, lastOffsetInNode(anchorNode), Position::PositionIsOffsetInAnchor);
+ return Position(anchorNode, anchorNode->length(), Position::PositionIsOffsetInAnchor);
return Position(anchorNode, Position::PositionIsAfterChildren);
}
inline int minOffsetForNode(Node* anchorNode, unsigned offset)
{
- if (anchorNode->isCharacterDataNode())
- return std::min<unsigned>(offset, anchorNode->maxCharacterOffset());
+ if (is<CharacterData>(*anchorNode))
+ return std::min(offset, downcast<CharacterData>(*anchorNode).length());
unsigned newOffset = 0;
for (Node* node = anchorNode->firstChild(); node && newOffset < offset; node = node->nextSibling())
newOffset++;
-
return newOffset;
}
inline bool offsetIsBeforeLastNodeOffset(unsigned offset, Node* anchorNode)
{
- if (anchorNode->isCharacterDataNode())
- return offset < static_cast<unsigned>(anchorNode->maxCharacterOffset());
+ if (is<CharacterData>(*anchorNode))
+ return offset < downcast<CharacterData>(*anchorNode).length();
unsigned currentOffset = 0;
for (Node* node = anchorNode->firstChild(); node && currentOffset < offset; node = node->nextSibling())
currentOffset++;
-
-
return offset < currentOffset;
}
Modified: trunk/Source/WebCore/dom/RangeBoundaryPoint.h (259929 => 259930)
--- trunk/Source/WebCore/dom/RangeBoundaryPoint.h 2020-04-11 15:14:34 UTC (rev 259929)
+++ trunk/Source/WebCore/dom/RangeBoundaryPoint.h 2020-04-11 15:35:04 UTC (rev 259930)
@@ -147,8 +147,8 @@
inline void RangeBoundaryPoint::setToEndOfNode(Ref<Node>&& container)
{
m_containerNode = WTFMove(container);
- if (m_containerNode->isCharacterDataNode()) {
- m_offsetInContainer = m_containerNode->maxCharacterOffset();
+ if (is<CharacterData>(*m_containerNode)) {
+ m_offsetInContainer = downcast<CharacterData>(*m_containerNode).length();
m_childBeforeBoundary = nullptr;
} else {
m_childBeforeBoundary = m_containerNode->lastChild();
Modified: trunk/Source/WebCore/editing/ApplyBlockElementCommand.cpp (259929 => 259930)
--- trunk/Source/WebCore/editing/ApplyBlockElementCommand.cpp 2020-04-11 15:14:34 UTC (rev 259929)
+++ trunk/Source/WebCore/editing/ApplyBlockElementCommand.cpp 2020-04-11 15:35:04 UTC (rev 259930)
@@ -175,8 +175,8 @@
static bool isNewLineAtPosition(const Position& position)
{
Node* textNode = position.containerNode();
- int offset = position.offsetInContainerNode();
- if (!is<Text>(textNode) || offset < 0 || offset >= textNode->maxCharacterOffset())
+ unsigned offset = position.offsetInContainerNode();
+ if (!is<Text>(textNode) || offset >= downcast<Text>(*textNode).length())
return false;
return downcast<Text>(*textNode).data()[offset] == '\n';
}
@@ -232,18 +232,18 @@
if (auto* endStyle = renderStyleOfEnclosingTextNode(end)) {
bool isEndAndEndOfLastParagraphOnSameNode = renderStyleOfEnclosingTextNode(m_endOfLastParagraph) && end.deprecatedNode() == m_endOfLastParagraph.deprecatedNode();
// Include \n at the end of line if we're at an empty paragraph
- if (endStyle->preserveNewline() && start == end && end.offsetInContainerNode() < end.containerNode()->maxCharacterOffset()) {
- int endOffset = end.offsetInContainerNode();
+ unsigned endOffset = end.offsetInContainerNode();
+ if (endStyle->preserveNewline() && start == end && endOffset < end.containerNode()->length()) {
if (!isNewLineAtPosition(end.previous()) && isNewLineAtPosition(end))
- end = Position(end.containerText(), endOffset + 1);
+ end = Position(end.containerText(), ++endOffset);
if (isEndAndEndOfLastParagraphOnSameNode && end.offsetInContainerNode() >= m_endOfLastParagraph.offsetInContainerNode())
m_endOfLastParagraph = end;
}
// If end is in the middle of a text node and the text node is editable, split.
- if (endStyle->userModify() != UserModify::ReadOnly && !endStyle->collapseWhiteSpace() && end.offsetInContainerNode() && end.offsetInContainerNode() < end.containerNode()->maxCharacterOffset()) {
+ if (endStyle->userModify() != UserModify::ReadOnly && !endStyle->collapseWhiteSpace() && endOffset && endOffset < end.containerNode()->length()) {
RefPtr<Text> endContainer = end.containerText();
- splitTextNode(*endContainer, end.offsetInContainerNode());
+ splitTextNode(*endContainer, endOffset);
if (is<Text>(endContainer) && !endContainer->previousSibling()) {
start = { };
end = { };
@@ -252,10 +252,10 @@
if (isStartAndEndOnSameNode)
start = firstPositionInOrBeforeNode(endContainer->previousSibling());
if (isEndAndEndOfLastParagraphOnSameNode) {
- if (m_endOfLastParagraph.offsetInContainerNode() == end.offsetInContainerNode())
+ if (static_cast<unsigned>(m_endOfLastParagraph.offsetInContainerNode()) == endOffset)
m_endOfLastParagraph = lastPositionInOrAfterNode(endContainer->previousSibling());
else
- m_endOfLastParagraph = Position(endContainer.get(), m_endOfLastParagraph.offsetInContainerNode() - end.offsetInContainerNode());
+ m_endOfLastParagraph = Position(endContainer.get(), m_endOfLastParagraph.offsetInContainerNode() - endOffset);
}
end = lastPositionInNode(endContainer->previousSibling());
}
Modified: trunk/Source/WebCore/editing/ApplyStyleCommand.cpp (259929 => 259930)
--- trunk/Source/WebCore/editing/ApplyStyleCommand.cpp 2020-04-11 15:14:34 UTC (rev 259929)
+++ trunk/Source/WebCore/editing/ApplyStyleCommand.cpp 2020-04-11 15:35:04 UTC (rev 259930)
@@ -1091,7 +1091,7 @@
// Move it to the next deep quivalent position to avoid removing the style from this node.
// e.g. if pushDownStart was at Position("hello", 5) in <b>hello<div>world</div></b>, we want Position("world", 0) instead.
auto* pushDownStartContainer = pushDownStart.containerNode();
- if (is<Text>(pushDownStartContainer) && pushDownStart.computeOffsetInContainerNode() == pushDownStartContainer->maxCharacterOffset())
+ if (is<Text>(pushDownStartContainer) && static_cast<unsigned>(pushDownStart.computeOffsetInContainerNode()) == downcast<Text>(*pushDownStartContainer).length())
pushDownStart = nextVisuallyDistinctCandidate(pushDownStart);
// If pushDownEnd is at the start of a text node, then this node is not fully selected.
// Move it to the previous deep equivalent position to avoid removing the style from this node.
Modified: trunk/Source/WebCore/editing/Editing.cpp (259929 => 259930)
--- trunk/Source/WebCore/editing/Editing.cpp 2020-04-11 15:14:34 UTC (rev 259929)
+++ trunk/Source/WebCore/editing/Editing.cpp 2020-04-11 15:35:04 UTC (rev 259930)
@@ -377,18 +377,11 @@
// on a Position before using it to create a DOM Range, or an exception will be thrown.
int lastOffsetForEditing(const Node& node)
{
- if (node.isCharacterDataNode())
- return node.maxCharacterOffset();
+ if (node.isCharacterDataNode() || node.hasChildNodes())
+ return node.length();
- if (node.hasChildNodes())
- return node.countChildNodes();
-
- // NOTE: This should preempt the countChildNodes() for, e.g., select nodes.
- // FIXME: What does the comment above mean?
- if (editingIgnoresContent(node))
- return 1;
-
- return 0;
+ // FIXME: Might be more helpful to return 1 for any node where editingIgnoresContent is true, even one that happens to have child nodes, like a select element with option node children.
+ return editingIgnoresContent(node) ? 1 : 0;
}
bool isAmbiguousBoundaryCharacter(UChar character)
Modified: trunk/Source/WebCore/editing/EditingStyle.cpp (259929 => 259930)
--- trunk/Source/WebCore/editing/EditingStyle.cpp 2020-04-11 15:14:34 UTC (rev 259929)
+++ trunk/Source/WebCore/editing/EditingStyle.cpp 2020-04-11 15:35:04 UTC (rev 259930)
@@ -1497,8 +1497,8 @@
// e.g. if pos was at Position("hello", 5) in <b>hello<div>world</div></b>, we want Position("world", 0) instead.
// We only do this for range because caret at Position("hello", 5) in <b>hello</b>world should give you font-weight: bold.
Node* positionNode = position.containerNode();
- if (selection.isRange() && positionNode && positionNode->isTextNode() && position.computeOffsetInContainerNode() == positionNode->maxCharacterOffset())
- position = nextVisuallyDistinctCandidate(position);
+ if (selection.isRange() && is<Text>(positionNode) && static_cast<unsigned>(position.computeOffsetInContainerNode()) == downcast<Text>(*positionNode).length())
+ position = nextVisuallyDistinctCandidate(position);
Element* element = position.element();
if (!element)
Modified: trunk/Source/WebCore/editing/InsertListCommand.cpp (259929 => 259930)
--- trunk/Source/WebCore/editing/InsertListCommand.cpp 2020-04-11 15:14:34 UTC (rev 259929)
+++ trunk/Source/WebCore/editing/InsertListCommand.cpp 2020-04-11 15:35:04 UTC (rev 259930)
@@ -256,7 +256,7 @@
if (rangeStartIsInList && newList)
currentSelection->setStart(*newList, 0);
if (rangeEndIsInList && newList)
- currentSelection->setEnd(*newList, lastOffsetInNode(newList.get()));
+ currentSelection->setEnd(*newList, newList->length());
setEndingSelection(VisiblePosition(firstPositionInNode(newList.get())));
Modified: trunk/Source/WebCore/editing/TextIterator.cpp (259929 => 259930)
--- trunk/Source/WebCore/editing/TextIterator.cpp 2020-04-11 15:14:34 UTC (rev 259929)
+++ trunk/Source/WebCore/editing/TextIterator.cpp 2020-04-11 15:35:04 UTC (rev 259930)
@@ -1134,7 +1134,7 @@
if (!endNode->isCharacterDataNode()) {
if (endOffset > 0 && endOffset <= endNode->countChildNodes()) {
endNode = endNode->traverseToChildAt(endOffset - 1);
- endOffset = lastOffsetInNode(endNode);
+ endOffset = endNode->length();
}
}
Modified: trunk/Source/WebCore/editing/VisibleUnits.cpp (259929 => 259930)
--- trunk/Source/WebCore/editing/VisibleUnits.cpp 2020-04-11 15:14:34 UTC (rev 259929)
+++ trunk/Source/WebCore/editing/VisibleUnits.cpp 2020-04-11 15:35:04 UTC (rev 259930)
@@ -637,7 +637,7 @@
return VisiblePosition(it.atEnd() ? searchRange->startPosition() : pos, DOWNSTREAM);
auto& node = it.atEnd() ? searchRange->startContainer() : it.range().start.container.get();
- if ((!suffixLength && node.isTextNode() && static_cast<int>(next) <= node.maxCharacterOffset()) || (node.renderer() && node.renderer()->isBR() && !next)) {
+ if ((!suffixLength && is<Text>(node) && next <= downcast<Text>(node).length()) || (node.renderer() && node.renderer()->isBR() && !next)) {
// The next variable contains a usable index into a text node
return VisiblePosition(createLegacyEditingPosition(&node, next), DOWNSTREAM);
}
Modified: trunk/Source/WebCore/html/HTMLTextFormControlElement.cpp (259929 => 259930)
--- trunk/Source/WebCore/html/HTMLTextFormControlElement.cpp 2020-04-11 15:14:34 UTC (rev 259929)
+++ trunk/Source/WebCore/html/HTMLTextFormControlElement.cpp 2020-04-11 15:35:04 UTC (rev 259930)
@@ -468,7 +468,7 @@
for (RefPtr<Node> node = innerText->firstChild(); node; node = NodeTraversal::next(*node, innerText.get())) {
ASSERT(!node->firstChild());
ASSERT(node->isTextNode() || node->hasTagName(brTag));
- int length = node->isTextNode() ? lastOffsetInNode(node.get()) : 1;
+ int length = is<Text>(*node) ? downcast<Text>(*node).length() : 1;
if (offset <= start && start <= offset + length)
setContainerAndOffsetForRange(node.get(), start - offset, startNode, start);
