Diff
Modified: trunk/LayoutTests/ChangeLog (220645 => 220646)
--- trunk/LayoutTests/ChangeLog 2017-08-14 06:55:11 UTC (rev 220645)
+++ trunk/LayoutTests/ChangeLog 2017-08-14 07:10:17 UTC (rev 220646)
@@ -1,3 +1,22 @@
+2017-08-14 Antti Koivisto <[email protected]>
+
+ [Render Tree Mutation] First letter should not mutate the render tree while in layout.
+ https://bugs.webkit.org/show_bug.cgi?id=163848
+ <rdar://problem/33402718>
+
+ Reviewed by Zalan Bujtas.
+
+ * fast/text-autosizing/ios/first-letter-expected.html: Added.
+
+ Turn into reftest for easier debugging and robustness.
+
+ * imported/blink/fast/css/first-letter-range-insert-expected.txt:
+
+ This is crash-or-assert test and the output change here doesn't matter.
+
+ * platform/ios/fast/text-autosizing/ios/first-letter-expected.txt: Removed.
+ * platform/mac/fast/text-autosizing/ios/first-letter-expected.txt: Removed.
+
2017-08-13 Manuel Rego Casasnovas <[email protected]>
Composition underline color is always black
Added: trunk/LayoutTests/fast/text-autosizing/ios/first-letter-expected.html (0 => 220646)
--- trunk/LayoutTests/fast/text-autosizing/ios/first-letter-expected.html (rev 0)
+++ trunk/LayoutTests/fast/text-autosizing/ios/first-letter-expected.html 2017-08-14 07:10:17 UTC (rev 220646)
@@ -0,0 +1,16 @@
+<html>
+<head>
+ <script>
+ if (window.internals) {
+ window.internals.settings.setTextAutosizingEnabled(true);
+ window.internals.settings.setTextAutosizingWindowSizeOverride(320, 480);
+ }
+ </script>
+</head>
+<body>
+ <p>The following lines should be identical.</p>
+ <p><span style="color:green">A</span>-Z</p>
+ <p><span style="color:green">A</span>-Z</p>
+ <p><span style="color:green">A</span>-Z</p>
+</body>
+</html>
Modified: trunk/LayoutTests/imported/blink/fast/css/first-letter-range-insert-expected.txt (220645 => 220646)
--- trunk/LayoutTests/imported/blink/fast/css/first-letter-range-insert-expected.txt 2017-08-14 06:55:11 UTC (rev 220645)
+++ trunk/LayoutTests/imported/blink/fast/css/first-letter-range-insert-expected.txt 2017-08-14 07:10:17 UTC (rev 220646)
@@ -1 +1,2 @@
+B
Deleted: trunk/LayoutTests/platform/ios/fast/text-autosizing/ios/first-letter-expected.txt (220645 => 220646)
--- trunk/LayoutTests/platform/ios/fast/text-autosizing/ios/first-letter-expected.txt 2017-08-14 06:55:11 UTC (rev 220645)
+++ trunk/LayoutTests/platform/ios/fast/text-autosizing/ios/first-letter-expected.txt 2017-08-14 07:10:17 UTC (rev 220646)
@@ -1,26 +0,0 @@
-layer at (0,0) size 800x600
- RenderView at (0,0) size 800x600
-layer at (0,0) size 800x600
- RenderBlock {HTML} at (0,0) size 800x600
- RenderBody {BODY} at (8,8) size 784x576
- RenderBlock {P} at (0,0) size 784x27
- RenderText {#text} at (0,0) size 366x26
- text run at (0,0) width 366: "The following lines should be identical."
- RenderBlock {P} at (0,43) size 784x27
- RenderInline {SPAN} at (0,0) size 17x26 [color=#008000]
- RenderText {#text} at (0,0) size 17x26
- text run at (0,0) width 17: "A"
- RenderText {#text} at (16,0) size 23x26
- text run at (16,0) width 23: "-Z"
- RenderBlock {P} at (0,86) size 784x27
- RenderInline (generated) at (0,0) size 17x26 [color=#008000]
- RenderText {#text} at (0,0) size 17x26
- text run at (0,0) width 17: "A"
- RenderText {#text} at (16,0) size 23x26
- text run at (16,0) width 23: "-Z"
- RenderBlock {P} at (0,129) size 784x27
- RenderBlock (floating) at (0,0) size 17x27 [color=#008000]
- RenderText {#text} at (0,0) size 17x26
- text run at (0,0) width 17: "A"
- RenderText {#text} at (16,0) size 23x26
- text run at (16,0) width 23: "-Z"
Deleted: trunk/LayoutTests/platform/mac/fast/text-autosizing/ios/first-letter-expected.txt (220645 => 220646)
--- trunk/LayoutTests/platform/mac/fast/text-autosizing/ios/first-letter-expected.txt 2017-08-14 06:55:11 UTC (rev 220645)
+++ trunk/LayoutTests/platform/mac/fast/text-autosizing/ios/first-letter-expected.txt 2017-08-14 07:10:17 UTC (rev 220646)
@@ -1,26 +0,0 @@
-layer at (0,0) size 800x600
- RenderView at (0,0) size 800x600
-layer at (0,0) size 800x600
- RenderBlock {HTML} at (0,0) size 800x600
- RenderBody {BODY} at (8,8) size 784x576
- RenderBlock {P} at (0,0) size 784x26
- RenderText {#text} at (0,0) size 366x26
- text run at (0,0) width 366: "The following lines should be identical."
- RenderBlock {P} at (0,42) size 784x26
- RenderInline {SPAN} at (0,0) size 17x26 [color=#008000]
- RenderText {#text} at (0,0) size 17x26
- text run at (0,0) width 17: "A"
- RenderText {#text} at (16,0) size 23x26
- text run at (16,0) width 23: "-Z"
- RenderBlock {P} at (0,84) size 784x26
- RenderInline (generated) at (0,0) size 17x26 [color=#008000]
- RenderText {#text} at (0,0) size 17x26
- text run at (0,0) width 17: "A"
- RenderText {#text} at (16,0) size 23x26
- text run at (16,0) width 23: "-Z"
- RenderBlock {P} at (0,126) size 784x26
- RenderBlock (floating) at (0,0) size 17x26 [color=#008000]
- RenderText {#text} at (0,0) size 17x26
- text run at (0,0) width 17: "A"
- RenderText {#text} at (16,0) size 23x26
- text run at (16,0) width 23: "-Z"
Modified: trunk/Source/WebCore/ChangeLog (220645 => 220646)
--- trunk/Source/WebCore/ChangeLog 2017-08-14 06:55:11 UTC (rev 220645)
+++ trunk/Source/WebCore/ChangeLog 2017-08-14 07:10:17 UTC (rev 220646)
@@ -1,3 +1,42 @@
+2017-08-14 Antti Koivisto <[email protected]>
+
+ [Render Tree Mutation] First letter should not mutate the render tree while in layout.
+ https://bugs.webkit.org/show_bug.cgi?id=163848
+
+ Reviewed by Zalan Bujtas.
+
+ RenderBlock::updateFirstLetter shouldn't be called during layout. Instead it should
+ be invoked by the RenderTreeUpdater.
+
+ With this future patches can move updateFirstLetter() and the related functions
+ completely out of the render tree.
+
+ * rendering/RenderBlock.cpp:
+ (WebCore::RenderBlock::layout):
+
+ No more updateFirstLetter calls during layout...
+
+ (WebCore::RenderBlock::computePreferredLogicalWidths):
+
+ ...or preferred width computation.
+
+ (WebCore::RenderBlock::updateFirstLetter):
+ * rendering/RenderBlock.h:
+ * rendering/RenderRubyRun.cpp:
+ (WebCore::RenderRubyRun::updateFirstLetter):
+ * rendering/RenderRubyRun.h:
+ * rendering/RenderTable.cpp:
+ (WebCore::RenderTable::updateFirstLetter):
+ * rendering/RenderTable.h:
+ * rendering/svg/RenderSVGText.cpp:
+ (WebCore::RenderSVGText::updateFirstLetter):
+ * rendering/svg/RenderSVGText.h:
+ * style/RenderTreeUpdater.cpp:
+ (WebCore::RenderTreeUpdater::popParent):
+
+ Call updateFirstLetter when closing the element. All of of descedant renderers are known here
+ so this can be resolved correctly.
+
2017-08-13 Manuel Rego Casasnovas <[email protected]>
Composition underline color is always black
Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (220645 => 220646)
--- trunk/Source/WebCore/rendering/RenderBlock.cpp 2017-08-14 06:55:11 UTC (rev 220645)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp 2017-08-14 07:10:17 UTC (rev 220646)
@@ -1060,9 +1060,6 @@
StackStats::LayoutCheckPoint layoutCheckPoint;
OverflowEventDispatcher dispatcher(this);
- // Update our first letter info now.
- updateFirstLetter();
-
// Table cells call layoutBlock directly, so don't add any logic here. Put code into
// layoutBlock().
layoutBlock(false);
@@ -2743,10 +2740,6 @@
{
ASSERT(preferredLogicalWidthsDirty());
- // FIXME: Do not even try to reshuffle first letter renderers when we are not in layout
- // until after webkit.org/b/163848 is fixed.
- updateFirstLetter(view().frameView().isInRenderTreeLayout() ? RenderTreeMutationIsAllowed::Yes : RenderTreeMutationIsAllowed::No);
-
m_minPreferredLogicalWidth = 0;
m_maxPreferredLogicalWidth = 0;
@@ -3342,8 +3335,10 @@
firstLetterContainer = nullptr;
}
-void RenderBlock::updateFirstLetter(RenderTreeMutationIsAllowed mutationAllowedOrNot)
+void RenderBlock::updateFirstLetter()
{
+ ASSERT_WITH_SECURITY_IMPLICATION(!view().layoutState());
+
RenderObject* firstLetterObj;
RenderElement* firstLetterContainer;
// FIXME: The first letter might be composed of a variety of code units, and therefore might
@@ -3363,12 +3358,6 @@
if (!is<RenderText>(*firstLetterObj))
return;
- if (mutationAllowedOrNot != RenderTreeMutationIsAllowed::Yes)
- return;
- // Our layout state is not valid for the repaints we are going to trigger by
- // adding and removing children of firstLetterContainer.
- LayoutStateDisabler layoutStateDisabler(view());
-
createFirstLetterRenderer(firstLetterContainer, downcast<RenderText>(firstLetterObj));
}
Modified: trunk/Source/WebCore/rendering/RenderBlock.h (220645 => 220646)
--- trunk/Source/WebCore/rendering/RenderBlock.h 2017-08-14 06:55:11 UTC (rev 220645)
+++ trunk/Source/WebCore/rendering/RenderBlock.h 2017-08-14 07:10:17 UTC (rev 220646)
@@ -263,8 +263,7 @@
LayoutUnit collapsedMarginBeforeForChild(const RenderBox& child) const;
LayoutUnit collapsedMarginAfterForChild(const RenderBox& child) const;
- enum class RenderTreeMutationIsAllowed { Yes, No };
- virtual void updateFirstLetter(RenderTreeMutationIsAllowed = RenderTreeMutationIsAllowed::Yes);
+ virtual void updateFirstLetter();
void getFirstLetter(RenderObject*& firstLetter, RenderElement*& firstLetterContainer, RenderObject* skipObject = nullptr);
virtual void scrollbarsChanged(bool /*horizontalScrollbarChanged*/, bool /*verticalScrollbarChanged*/) { }
Modified: trunk/Source/WebCore/rendering/RenderRubyRun.cpp (220645 => 220646)
--- trunk/Source/WebCore/rendering/RenderRubyRun.cpp 2017-08-14 06:55:11 UTC (rev 220645)
+++ trunk/Source/WebCore/rendering/RenderRubyRun.cpp 2017-08-14 07:10:17 UTC (rev 220646)
@@ -101,7 +101,7 @@
return 0;
}
-void RenderRubyRun::updateFirstLetter(RenderTreeMutationIsAllowed)
+void RenderRubyRun::updateFirstLetter()
{
}
Modified: trunk/Source/WebCore/rendering/RenderRubyRun.h (220645 => 220646)
--- trunk/Source/WebCore/rendering/RenderRubyRun.h 2017-08-14 06:55:11 UTC (rev 220645)
+++ trunk/Source/WebCore/rendering/RenderRubyRun.h 2017-08-14 07:10:17 UTC (rev 220646)
@@ -60,7 +60,7 @@
void removeChild(RenderObject&) override;
RenderBlock* firstLineBlock() const override;
- void updateFirstLetter(RenderTreeMutationIsAllowed = RenderTreeMutationIsAllowed::Yes) override;
+ void updateFirstLetter() override;
void getOverhang(bool firstLine, RenderObject* startRenderer, RenderObject* endRenderer, float& startOverhang, float& endOverhang) const;
Modified: trunk/Source/WebCore/rendering/RenderTable.cpp (220645 => 220646)
--- trunk/Source/WebCore/rendering/RenderTable.cpp 2017-08-14 06:55:11 UTC (rev 220645)
+++ trunk/Source/WebCore/rendering/RenderTable.cpp 2017-08-14 07:10:17 UTC (rev 220646)
@@ -1485,7 +1485,7 @@
return nullptr;
}
-void RenderTable::updateFirstLetter(RenderTreeMutationIsAllowed)
+void RenderTable::updateFirstLetter()
{
}
Modified: trunk/Source/WebCore/rendering/RenderTable.h (220645 => 220646)
--- trunk/Source/WebCore/rendering/RenderTable.h 2017-08-14 06:55:11 UTC (rev 220645)
+++ trunk/Source/WebCore/rendering/RenderTable.h 2017-08-14 07:10:17 UTC (rev 220646)
@@ -302,7 +302,7 @@
void invalidateCachedColumnOffsets();
RenderBlock* firstLineBlock() const final;
- void updateFirstLetter(RenderTreeMutationIsAllowed = RenderTreeMutationIsAllowed::Yes) final;
+ void updateFirstLetter() final;
void updateLogicalWidth() final;
Modified: trunk/Source/WebCore/rendering/RenderTableCell.cpp (220645 => 220646)
--- trunk/Source/WebCore/rendering/RenderTableCell.cpp 2017-08-14 06:55:11 UTC (rev 220645)
+++ trunk/Source/WebCore/rendering/RenderTableCell.cpp 2017-08-14 07:10:17 UTC (rev 220646)
@@ -260,7 +260,6 @@
void RenderTableCell::layout()
{
StackStats::LayoutCheckPoint layoutCheckPoint;
- updateFirstLetter();
int oldCellBaseline = cellBaselinePosition();
layoutBlock(cellWidthChanged());
Modified: trunk/Source/WebCore/rendering/RenderTextFragment.cpp (220645 => 220646)
--- trunk/Source/WebCore/rendering/RenderTextFragment.cpp 2017-08-14 06:55:11 UTC (rev 220645)
+++ trunk/Source/WebCore/rendering/RenderTextFragment.cpp 2017-08-14 07:10:17 UTC (rev 220646)
@@ -68,10 +68,8 @@
{
RenderText::styleDidChange(diff, oldStyle);
- if (RenderBlock* block = blockForAccompanyingFirstLetter()) {
+ if (RenderBlock* block = blockForAccompanyingFirstLetter())
block->mutableStyle().removeCachedPseudoStyle(FIRST_LETTER);
- block->updateFirstLetter();
- }
}
void RenderTextFragment::willBeDestroyed()
Modified: trunk/Source/WebCore/rendering/TextAutoSizing.cpp (220645 => 220646)
--- trunk/Source/WebCore/rendering/TextAutoSizing.cpp 2017-08-14 06:55:11 UTC (rev 220645)
+++ trunk/Source/WebCore/rendering/TextAutoSizing.cpp 2017-08-14 07:10:17 UTC (rev 220646)
@@ -32,8 +32,10 @@
#include "Document.h"
#include "FontCascade.h"
#include "Logging.h"
+#include "RenderBlock.h"
#include "RenderListMarker.h"
#include "RenderText.h"
+#include "RenderTextFragment.h"
#include "StyleResolver.h"
namespace WebCore {
@@ -155,6 +157,17 @@
parentRenderer->setStyle(WTFMove(newParentStyle));
}
+ // FIXME: All render tree mutations should be done via RenderTreeUpdater.
+ for (auto& node : m_autoSizedNodes) {
+ auto& textRenderer = *node->renderer();
+ if (!is<RenderTextFragment>(textRenderer))
+ continue;
+ auto* block = downcast<RenderTextFragment>(textRenderer).blockForAccompanyingFirstLetter();
+ if (!block)
+ continue;
+ block->updateFirstLetter();
+ }
+
return stillHasNodes;
}
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGText.cpp (220645 => 220646)
--- trunk/Source/WebCore/rendering/svg/RenderSVGText.cpp 2017-08-14 06:55:11 UTC (rev 220645)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGText.cpp 2017-08-14 07:10:17 UTC (rev 220646)
@@ -544,7 +544,7 @@
// Fix for <rdar://problem/8048875>. We should not render :first-letter CSS Style
// in a SVG text element context.
-void RenderSVGText::updateFirstLetter(RenderTreeMutationIsAllowed)
+void RenderSVGText::updateFirstLetter()
{
}
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGText.h (220645 => 220646)
--- trunk/Source/WebCore/rendering/svg/RenderSVGText.h 2017-08-14 06:55:11 UTC (rev 220645)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGText.h 2017-08-14 07:10:17 UTC (rev 220646)
@@ -91,7 +91,7 @@
std::unique_ptr<RootInlineBox> createRootInlineBox() override;
RenderBlock* firstLineBlock() const override;
- void updateFirstLetter(RenderTreeMutationIsAllowed = RenderTreeMutationIsAllowed::Yes) override;
+ void updateFirstLetter() override;
bool shouldHandleSubtreeMutations() const;
Modified: trunk/Source/WebCore/style/RenderTreeUpdater.cpp (220645 => 220646)
--- trunk/Source/WebCore/style/RenderTreeUpdater.cpp 2017-08-14 06:55:11 UTC (rev 220645)
+++ trunk/Source/WebCore/style/RenderTreeUpdater.cpp 2017-08-14 07:10:17 UTC (rev 220646)
@@ -232,7 +232,11 @@
if (parent.element) {
updateBeforeOrAfterPseudoElement(*parent.element, AFTER);
- if (parent.element->hasCustomStyleResolveCallbacks() && parent.styleChange == Style::Detach && parent.element->renderer())
+ auto* renderer = parent.element->renderer();
+ if (is<RenderBlock>(renderer))
+ downcast<RenderBlock>(*renderer).updateFirstLetter();
+
+ if (parent.element->hasCustomStyleResolveCallbacks() && parent.styleChange == Style::Detach && renderer)
parent.element->didAttachRenderers();
}
m_parentStack.removeLast();
