Diff
Modified: trunk/Source/WebCore/ChangeLog (280173 => 280174)
--- trunk/Source/WebCore/ChangeLog 2021-07-22 11:48:39 UTC (rev 280173)
+++ trunk/Source/WebCore/ChangeLog 2021-07-22 16:27:33 UTC (rev 280174)
@@ -1,3 +1,32 @@
+2021-07-22 Frédéric Wang <[email protected]>
+
+ nullptr crash in ApplyStyleCommand::applyRelativeFontStyleChange
+ https://bugs.webkit.org/show_bug.cgi?id=223974
+
+ Reviewed by Darin Adler.
+
+ WebCore::documentOrder does not handle well elements like <summary> that contains a
+ shadow substree. This is causing assertion failures in debug build when setting start/end
+ selection and nullptr crashes in release build when trying to browse selection between these
+ start and end nodes. This patch fixes that issue by switching to shadow including tree order
+ for these particular cases. It introduces a generic treeOrder<TreeType>(a, b) function that
+ can be used for TreeType = ShadowIncludingTree as well as by WebCore::documentOrder.
+
+ * dom/Node.cpp: Explicitly instantiate commonInclusiveAncestor<ShadowIncludingTree> so that it can be used
+ by WebCore::treeOrder.
+ * dom/Position.cpp: Explicitly instantiate templates.
+ (WebCore::treeOrder): Convert documentOrder to a template parametrized by TreeType.
+ (WebCore::documentOrder): Implement it with treeOrder<ComposedTree>.
+ * dom/Position.h: Delcare new template.
+ * editing/ApplyStyleCommand.cpp:
+ (WebCore::ApplyStyleCommand::updateStartEnd): Use treeOrder<ShadowIncludingTree>.
+ (WebCore::ApplyStyleCommand::removeInlineStyle): Ditto.
+ * editing/VisiblePosition.cpp:
+ (WebCore::documentOrder): Use treeOrder<ShadowIncludingTree>.
+ * editing/VisibleSelection.cpp:
+ (WebCore::VisibleSelection::setBaseAndExtentToDeepEquivalents): Use treeOrder<ShadowIncludingTree>.
+ (WebCore::VisibleSelection::setWithoutValidation): Ditto.
+
2021-07-22 Martin Robinson <[email protected]>
[css-scroll-snap] Pass the full target point when selecting a snap offset
Modified: trunk/Source/WebCore/dom/Node.cpp (280173 => 280174)
--- trunk/Source/WebCore/dom/Node.cpp 2021-07-22 11:48:39 UTC (rev 280173)
+++ trunk/Source/WebCore/dom/Node.cpp 2021-07-22 16:27:33 UTC (rev 280174)
@@ -2684,6 +2684,7 @@
template Node* commonInclusiveAncestor<Tree>(const Node&, const Node&);
template Node* commonInclusiveAncestor<ComposedTree>(const Node&, const Node&);
+template Node* commonInclusiveAncestor<ShadowIncludingTree>(const Node&, const Node&);
static bool isSiblingSubsequent(const Node& siblingA, const Node& siblingB)
{
Modified: trunk/Source/WebCore/dom/Position.cpp (280173 => 280174)
--- trunk/Source/WebCore/dom/Position.cpp 2021-07-22 11:48:39 UTC (rev 280173)
+++ trunk/Source/WebCore/dom/Position.cpp 2021-07-22 16:27:33 UTC (rev 280174)
@@ -1609,7 +1609,7 @@
return BoundaryPoint { container.releaseNonNull(), static_cast<unsigned>(position.computeOffsetInContainerNode()) };
}
-PartialOrdering documentOrder(const Position& a, const Position& b)
+template<TreeType treeType> PartialOrdering treeOrder(const Position& a, const Position& b)
{
if (a.isNull() || b.isNull())
return a.isNull() && b.isNull() ? PartialOrdering::equivalent : PartialOrdering::unordered;
@@ -1618,7 +1618,7 @@
auto bContainer = b.containerNode();
if (!aContainer || !bContainer) {
- if (!commonInclusiveAncestor<ComposedTree>(*a.anchorNode(), *b.anchorNode()))
+ if (!commonInclusiveAncestor<treeType>(*a.anchorNode(), *b.anchorNode()))
return PartialOrdering::unordered;
if (!aContainer && !bContainer && a.anchorType() == b.anchorType())
return PartialOrdering::equivalent;
@@ -1629,9 +1629,17 @@
// FIXME: Avoid computing node offset for cases where we don't need to.
- return treeOrder<ComposedTree>(*makeBoundaryPoint(a), *makeBoundaryPoint(b));
+ return treeOrder<treeType>(*makeBoundaryPoint(a), *makeBoundaryPoint(b));
}
+PartialOrdering documentOrder(const Position& a, const Position& b)
+{
+ return treeOrder<ComposedTree>(a, b);
+}
+
+template PartialOrdering treeOrder<ComposedTree>(const Position&, const Position&);
+template PartialOrdering treeOrder<ShadowIncludingTree>(const Position&, const Position&);
+
} // namespace WebCore
#if ENABLE(TREE_DEBUGGING)
Modified: trunk/Source/WebCore/dom/Position.h (280173 => 280174)
--- trunk/Source/WebCore/dom/Position.h 2021-07-22 11:48:39 UTC (rev 280173)
+++ trunk/Source/WebCore/dom/Position.h 2021-07-22 16:27:33 UTC (rev 280174)
@@ -220,6 +220,7 @@
bool operator==(const Position&, const Position&);
bool operator!=(const Position&, const Position&);
+template<TreeType treeType> PartialOrdering treeOrder(const Position&, const Position&);
WEBCORE_EXPORT PartialOrdering documentOrder(const Position&, const Position&);
bool operator<(const Position&, const Position&);
bool operator>(const Position&, const Position&);
Modified: trunk/Source/WebCore/editing/ApplyStyleCommand.cpp (280173 => 280174)
--- trunk/Source/WebCore/editing/ApplyStyleCommand.cpp 2021-07-22 11:48:39 UTC (rev 280173)
+++ trunk/Source/WebCore/editing/ApplyStyleCommand.cpp 2021-07-22 16:27:33 UTC (rev 280174)
@@ -173,7 +173,7 @@
void ApplyStyleCommand::updateStartEnd(const Position& newStart, const Position& newEnd)
{
- ASSERT(!(newStart > newEnd));
+ ASSERT(!is_gt(treeOrder<ShadowIncludingTree>(newStart, newEnd)));
if (!m_useEndingSelection && (newStart != m_start || newEnd != m_end))
m_useEndingSelection = true;
@@ -1074,7 +1074,7 @@
ASSERT(end.isNotNull());
ASSERT(start.anchorNode()->isConnected());
ASSERT(end.anchorNode()->isConnected());
- ASSERT(start <= end);
+ ASSERT(is_lteq(treeOrder<ShadowIncludingTree>(start, end)));
// FIXME: We should assert that start/end are not in the middle of a text node.
Position pushDownStart = start.downstream();
Modified: trunk/Source/WebCore/editing/VisiblePosition.cpp (280173 => 280174)
--- trunk/Source/WebCore/editing/VisiblePosition.cpp 2021-07-22 11:48:39 UTC (rev 280173)
+++ trunk/Source/WebCore/editing/VisiblePosition.cpp 2021-07-22 16:27:33 UTC (rev 280174)
@@ -799,7 +799,7 @@
PartialOrdering documentOrder(const VisiblePosition& a, const VisiblePosition& b)
{
// FIXME: Should two positions with different affinity be considered equivalent or not?
- return documentOrder(a.deepEquivalent(), b.deepEquivalent());
+ return treeOrder<ComposedTree>(a.deepEquivalent(), b.deepEquivalent());
}
bool intersects(const VisiblePositionRange& a, const VisiblePositionRange& b)
Modified: trunk/Source/WebCore/editing/VisibleSelection.cpp (280173 => 280174)
--- trunk/Source/WebCore/editing/VisibleSelection.cpp 2021-07-22 11:48:39 UTC (rev 280173)
+++ trunk/Source/WebCore/editing/VisibleSelection.cpp 2021-07-22 16:27:33 UTC (rev 280174)
@@ -251,7 +251,7 @@
if (m_focus.isNull())
m_focus = m_anchor;
- m_anchorIsFirst = m_anchor <= m_focus;
+ m_anchorIsFirst = is_lteq(treeOrder<ShadowIncludingTree>(m_anchor, m_focus));
m_base = VisiblePosition(m_anchor, m_affinity).deepEquivalent();
if (m_anchor == m_focus)
@@ -461,7 +461,7 @@
ASSERT(m_affinity == Affinity::Downstream);
m_anchor = anchor;
m_focus = focus;
- m_anchorIsFirst = m_anchor <= m_focus;
+ m_anchorIsFirst = is_lteq(treeOrder<ShadowIncludingTree>(m_anchor, m_focus));
m_base = anchor;
m_extent = focus;
m_start = m_anchorIsFirst ? anchor : focus;
Modified: trunk/Tools/ChangeLog (280173 => 280174)
--- trunk/Tools/ChangeLog 2021-07-22 11:48:39 UTC (rev 280173)
+++ trunk/Tools/ChangeLog 2021-07-22 16:27:33 UTC (rev 280174)
@@ -1,3 +1,12 @@
+2021-07-22 Frédéric Wang <[email protected]>
+
+ nullptr crash in ApplyStyleCommand::applyRelativeFontStyleChange
+ https://bugs.webkit.org/show_bug.cgi?id=223974
+
+ Reviewed by Darin Adler.
+
+ * TestWebKitAPI/Tests/WebCore/DocumentOrder.cpp: Update FIXME.
+
2021-07-22 Philippe Normand <[email protected]>
[GLib] Expose API to access/modify capture devices states
Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/DocumentOrder.cpp (280173 => 280174)
--- trunk/Tools/TestWebKitAPI/Tests/WebCore/DocumentOrder.cpp 2021-07-22 11:48:39 UTC (rev 280173)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/DocumentOrder.cpp 2021-07-22 16:27:33 UTC (rev 280174)
@@ -37,7 +37,7 @@
#include <WebCore/TextControlInnerElements.h>
#include <WebCore/WebKitFontFamilyNames.h>
-// FIXME: Expose the functions tested here in WebKit internals object, then replace this test with one written in _javascript_.
+// FIXME(https://webkit.org/b/228175): Expose the functions tested here in WebKit internals object, then replace this test with one written in _javascript_.
// FIXME: When doing the above, don't forget to remove the many WEBCORE_EXPORT that were added so we could compile and link this test.
#define EXPECT_BOTH(a, b, forward, reversed) do { EXPECT_STREQ(string(documentOrder(a, b)), forward); EXPECT_STREQ(string(documentOrder(b, a)), reversed); } while (0)
