<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[280174] trunk</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/280174">280174</a></dd>
<dt>Author</dt> <dd>commit-queue@webkit.org</dd>
<dt>Date</dt> <dd>2021-07-22 09:27:33 -0700 (Thu, 22 Jul 2021)</dd>
</dl>

<h3>Log Message</h3>
<pre>nullptr crash in ApplyStyleCommand::applyRelativeFontStyleChange
https://bugs.webkit.org/show_bug.cgi?id=223974

Patch by Frédéric Wang <fwang@igalia.com> on 2021-07-22
Reviewed by Darin Adler.

Source/WebCore:

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.

Tools:

* TestWebKitAPI/Tests/WebCore/DocumentOrder.cpp: Update FIXME.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoredomNodecpp">trunk/Source/WebCore/dom/Node.cpp</a></li>
<li><a href="#trunkSourceWebCoredomPositioncpp">trunk/Source/WebCore/dom/Position.cpp</a></li>
<li><a href="#trunkSourceWebCoredomPositionh">trunk/Source/WebCore/dom/Position.h</a></li>
<li><a href="#trunkSourceWebCoreeditingApplyStyleCommandcpp">trunk/Source/WebCore/editing/ApplyStyleCommand.cpp</a></li>
<li><a href="#trunkSourceWebCoreeditingVisiblePositioncpp">trunk/Source/WebCore/editing/VisiblePosition.cpp</a></li>
<li><a href="#trunkSourceWebCoreeditingVisibleSelectioncpp">trunk/Source/WebCore/editing/VisibleSelection.cpp</a></li>
<li><a href="#trunkToolsChangeLog">trunk/Tools/ChangeLog</a></li>
<li><a href="#trunkToolsTestWebKitAPITestsWebCoreDocumentOrdercpp">trunk/Tools/TestWebKitAPI/Tests/WebCore/DocumentOrder.cpp</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (280173 => 280174)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,32 @@
</span><ins>+2021-07-22  Frédéric Wang  <fwang@igalia.com>
+
+        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.
+
</ins><span class="cx"> 2021-07-22  Martin Robinson  <mrobinson@webkit.org>
</span><span class="cx"> 
</span><span class="cx">         [css-scroll-snap] Pass the full target point when selecting a snap offset
</span></span></pre></div>
<a id="trunkSourceWebCoredomNodecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/Node.cpp (280173 => 280174)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -2684,6 +2684,7 @@
</span><span class="cx"> 
</span><span class="cx"> template Node* commonInclusiveAncestor<Tree>(const Node&, const Node&);
</span><span class="cx"> template Node* commonInclusiveAncestor<ComposedTree>(const Node&, const Node&);
</span><ins>+template Node* commonInclusiveAncestor<ShadowIncludingTree>(const Node&, const Node&);
</ins><span class="cx"> 
</span><span class="cx"> static bool isSiblingSubsequent(const Node& siblingA, const Node& siblingB)
</span><span class="cx"> {
</span></span></pre></div>
<a id="trunkSourceWebCoredomPositioncpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/Position.cpp (280173 => 280174)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1609,7 +1609,7 @@
</span><span class="cx">     return BoundaryPoint { container.releaseNonNull(), static_cast<unsigned>(position.computeOffsetInContainerNode()) };
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-PartialOrdering documentOrder(const Position& a, const Position& b)
</del><ins>+template<TreeType treeType> PartialOrdering treeOrder(const Position& a, const Position& b)
</ins><span class="cx"> {
</span><span class="cx">     if (a.isNull() || b.isNull())
</span><span class="cx">         return a.isNull() && b.isNull() ? PartialOrdering::equivalent : PartialOrdering::unordered;
</span><span class="lines">@@ -1618,7 +1618,7 @@
</span><span class="cx">     auto bContainer = b.containerNode();
</span><span class="cx"> 
</span><span class="cx">     if (!aContainer || !bContainer) {
</span><del>-        if (!commonInclusiveAncestor<ComposedTree>(*a.anchorNode(), *b.anchorNode()))
</del><ins>+        if (!commonInclusiveAncestor<treeType>(*a.anchorNode(), *b.anchorNode()))
</ins><span class="cx">             return PartialOrdering::unordered;
</span><span class="cx">         if (!aContainer && !bContainer && a.anchorType() == b.anchorType())
</span><span class="cx">             return PartialOrdering::equivalent;
</span><span class="lines">@@ -1629,9 +1629,17 @@
</span><span class="cx"> 
</span><span class="cx">     // FIXME: Avoid computing node offset for cases where we don't need to.
</span><span class="cx"> 
</span><del>-    return treeOrder<ComposedTree>(*makeBoundaryPoint(a), *makeBoundaryPoint(b));
</del><ins>+    return treeOrder<treeType>(*makeBoundaryPoint(a), *makeBoundaryPoint(b));
</ins><span class="cx"> }
</span><span class="cx"> 
</span><ins>+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&);
+
</ins><span class="cx"> } // namespace WebCore
</span><span class="cx"> 
</span><span class="cx"> #if ENABLE(TREE_DEBUGGING)
</span></span></pre></div>
<a id="trunkSourceWebCoredomPositionh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/Position.h (280173 => 280174)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -220,6 +220,7 @@
</span><span class="cx"> bool operator==(const Position&, const Position&);
</span><span class="cx"> bool operator!=(const Position&, const Position&);
</span><span class="cx"> 
</span><ins>+template<TreeType treeType> PartialOrdering treeOrder(const Position&, const Position&);
</ins><span class="cx"> WEBCORE_EXPORT PartialOrdering documentOrder(const Position&, const Position&);
</span><span class="cx"> bool operator<(const Position&, const Position&);
</span><span class="cx"> bool operator>(const Position&, const Position&);
</span></span></pre></div>
<a id="trunkSourceWebCoreeditingApplyStyleCommandcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/editing/ApplyStyleCommand.cpp (280173 => 280174)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -173,7 +173,7 @@
</span><span class="cx"> 
</span><span class="cx"> void ApplyStyleCommand::updateStartEnd(const Position& newStart, const Position& newEnd)
</span><span class="cx"> {
</span><del>-    ASSERT(!(newStart > newEnd));
</del><ins>+    ASSERT(!is_gt(treeOrder<ShadowIncludingTree>(newStart, newEnd)));
</ins><span class="cx"> 
</span><span class="cx">     if (!m_useEndingSelection && (newStart != m_start || newEnd != m_end))
</span><span class="cx">         m_useEndingSelection = true;
</span><span class="lines">@@ -1074,7 +1074,7 @@
</span><span class="cx">     ASSERT(end.isNotNull());
</span><span class="cx">     ASSERT(start.anchorNode()->isConnected());
</span><span class="cx">     ASSERT(end.anchorNode()->isConnected());
</span><del>-    ASSERT(start <= end);
</del><ins>+    ASSERT(is_lteq(treeOrder<ShadowIncludingTree>(start, end)));
</ins><span class="cx">     // FIXME: We should assert that start/end are not in the middle of a text node.
</span><span class="cx"> 
</span><span class="cx">     Position pushDownStart = start.downstream();
</span></span></pre></div>
<a id="trunkSourceWebCoreeditingVisiblePositioncpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/editing/VisiblePosition.cpp (280173 => 280174)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -799,7 +799,7 @@
</span><span class="cx"> PartialOrdering documentOrder(const VisiblePosition& a, const VisiblePosition& b)
</span><span class="cx"> {
</span><span class="cx">     // FIXME: Should two positions with different affinity be considered equivalent or not?
</span><del>-    return documentOrder(a.deepEquivalent(), b.deepEquivalent());
</del><ins>+    return treeOrder<ComposedTree>(a.deepEquivalent(), b.deepEquivalent());
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> bool intersects(const VisiblePositionRange& a, const VisiblePositionRange& b)
</span></span></pre></div>
<a id="trunkSourceWebCoreeditingVisibleSelectioncpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/editing/VisibleSelection.cpp (280173 => 280174)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -251,7 +251,7 @@
</span><span class="cx">     if (m_focus.isNull())
</span><span class="cx">         m_focus = m_anchor;
</span><span class="cx"> 
</span><del>-    m_anchorIsFirst = m_anchor <= m_focus;
</del><ins>+    m_anchorIsFirst = is_lteq(treeOrder<ShadowIncludingTree>(m_anchor, m_focus));
</ins><span class="cx"> 
</span><span class="cx">     m_base = VisiblePosition(m_anchor, m_affinity).deepEquivalent();
</span><span class="cx">     if (m_anchor == m_focus)
</span><span class="lines">@@ -461,7 +461,7 @@
</span><span class="cx">     ASSERT(m_affinity == Affinity::Downstream);
</span><span class="cx">     m_anchor = anchor;
</span><span class="cx">     m_focus = focus;
</span><del>-    m_anchorIsFirst = m_anchor <= m_focus;
</del><ins>+    m_anchorIsFirst = is_lteq(treeOrder<ShadowIncludingTree>(m_anchor, m_focus));
</ins><span class="cx">     m_base = anchor;
</span><span class="cx">     m_extent = focus;
</span><span class="cx">     m_start = m_anchorIsFirst ? anchor : focus;
</span></span></pre></div>
<a id="trunkToolsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Tools/ChangeLog (280173 => 280174)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/ChangeLog    2021-07-22 11:48:39 UTC (rev 280173)
+++ trunk/Tools/ChangeLog       2021-07-22 16:27:33 UTC (rev 280174)
</span><span class="lines">@@ -1,3 +1,12 @@
</span><ins>+2021-07-22  Frédéric Wang  <fwang@igalia.com>
+
+        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.
+
</ins><span class="cx"> 2021-07-22  Philippe Normand  <pnormand@igalia.com>
</span><span class="cx"> 
</span><span class="cx">         [GLib] Expose API to access/modify capture devices states
</span></span></pre></div>
<a id="trunkToolsTestWebKitAPITestsWebCoreDocumentOrdercpp"></a>
<div class="modfile"><h4>Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/DocumentOrder.cpp (280173 => 280174)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -37,7 +37,7 @@
</span><span class="cx"> #include <WebCore/TextControlInnerElements.h>
</span><span class="cx"> #include <WebCore/WebKitFontFamilyNames.h>
</span><span class="cx"> 
</span><del>-// FIXME: Expose the functions tested here in WebKit internals object, then replace this test with one written in JavaScript.
</del><ins>+// FIXME(https://webkit.org/b/228175): Expose the functions tested here in WebKit internals object, then replace this test with one written in JavaScript.
</ins><span class="cx"> // 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.
</span><span class="cx"> 
</span><span class="cx"> #define EXPECT_BOTH(a, b, forward, reversed) do { EXPECT_STREQ(string(documentOrder(a, b)), forward); EXPECT_STREQ(string(documentOrder(b, a)), reversed); } while (0)
</span></span></pre>
</div>
</div>

</body>
</html>