Changeset 274398 in webkit

Timestamp:

Mar 14, 2021, 7:46:34 AM (5 years ago)

Author:

Alan Bujtas

Message:

RenderLineBreak should stay inline level box even when display property says otherwise.
​https://bugs.webkit.org/show_bug.cgi?id=223151
<rdar://74339837>

Reviewed by Antti Koivisto.

In this patch we make sure that the RenderLineBreak object always stays inline.

While the display property applies to all elements (​https://drafts.csswg.org/css-display/#the-display-properties), the line
break element (<br>) seems to be an exception and all browsers handle <br style="display: block"> still as an inline level element.

Renderers can (and do see RenderSVGInline::updateFromStyle) diverge from this computed display value and say they always want to
be inline level boxes. This is ensured by RenderObject::setInline/isInline function pair.

Initially there were 2 setInline() calls:
One in RenderObject::setStyle(). It dealt with "block to inline" transition and
one call in RenderBox::setStyle().
In these functions we simply set the renderer's "inInline" bit to the computed style value (setInline(style.isDisplayInlineType)).
However derived renderer classes were able to override it by implementing the ::setStyle function and
explicitly set the "isInline" bit to true/false.

Over the years the second (RenderBox::setStyle) call transitioned to RenderBoxModelObject::updateFromStyle()
and the first (RenderObject::setStyle) call got moved to normalizeTreeAfterStyleChange() and their order got flipped.

Derived renderer classes can still explicitly set the "isInline" bit by implementing the ::updateFromStyle() function
(this is similar to what we had with ::setStyle).
However since the order is flipped, the "setInline()" call in normalizeTreeAfterStyleChange() now runs after
updateFromStyle() and it may re-reset the "isInline" bit back to the computed value (hence the FIXME).

The "setInline" call in normalizeTreeAfterStyleChange() is not needed anymore since the "isInline" bit is already updated by
the time we get here (as opposed to when it was initially introduced in RenderObject::setStyle, see above).

• rendering/RenderLineBreak.cpp:

(WebCore::RenderLineBreak::updateFromStyle):

• rendering/updating/RenderTreeBuilder.cpp:

(WebCore::RenderTreeBuilder::normalizeTreeAfterStyleChange):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• rendering/RenderLineBreak.cpp (modified) (1 diff)
• rendering/updating/RenderTreeBuilder.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-03-14  Zalan Bujtas  <zalan@apple.com>
+
+        RenderLineBreak should stay inline level box even when display property says otherwise.
+        https://bugs.webkit.org/show_bug.cgi?id=223151
+        <rdar://74339837>
+
+        Reviewed by Antti Koivisto.
+
+        In this patch we make sure that the RenderLineBreak object always stays inline.
+
+        While the display property applies to all elements (https://drafts.csswg.org/css-display/#the-display-properties), the line
+        break element (<br>) seems to be an exception and all browsers handle <br style="display: block"> still as an inline level element.
+
+        Renderers can (and do see RenderSVGInline::updateFromStyle) diverge from this computed display value and say they always want to
+        be inline level boxes. This is ensured by RenderObject::setInline/isInline function pair.
+
+        Initially there were 2 setInline() calls:
+        One in RenderObject::setStyle(). It dealt with "block to inline" transition and
+        one call in RenderBox::setStyle().
+        In these functions we simply set the renderer's "inInline" bit to the computed style value (setInline(style.isDisplayInlineType)).
+        However derived renderer classes were able to override it by implementing the ::setStyle function and
+        explicitly set the "isInline" bit to true/false.
+
+        Over the years the second (RenderBox::setStyle) call transitioned to RenderBoxModelObject::updateFromStyle()
+        and the first (RenderObject::setStyle) call got moved to normalizeTreeAfterStyleChange() and their order got flipped.
+
+        Derived renderer classes can still explicitly set the "isInline" bit by implementing the ::updateFromStyle() function
+        (this is similar to what we had with ::setStyle).
+        However since the order is flipped, the "setInline()" call in normalizeTreeAfterStyleChange() now runs after
+        updateFromStyle() and it may re-reset the "isInline" bit back to the computed value (hence the FIXME).
+
+        The "setInline" call in normalizeTreeAfterStyleChange() is not needed anymore since the "isInline" bit is already updated by
+        the time we get here (as opposed to when it was initially introduced in RenderObject::setStyle, see above).
+
+        * rendering/RenderLineBreak.cpp:
+        (WebCore::RenderLineBreak::updateFromStyle):
+        * rendering/updating/RenderTreeBuilder.cpp:
+        (WebCore::RenderTreeBuilder::normalizeTreeAfterStyleChange):
+
 2021-03-13  Wenson Hsieh  <wenson_hsieh@apple.com>
 

trunk/Source/WebCore/rendering/RenderLineBreak.cpp

@@ -175 +175 @@
 {
     m_cachedLineHeight = invalidLineHeight;
+    RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(isInline());
 }
 

trunk/Source/WebCore/rendering/updating/RenderTreeBuilder.cpp

@@ -579 +579 @@
         // having an impact. See if there is a mismatch between the parent flow's
         // childrenInline() state and our state.
-        // FIXME(186894): startsAffectingParent has clearly nothing to do with resetting the inline state.
-        if (!is<RenderSVGInline>(renderer))
-            renderer.setInline(renderer.style().isDisplayInlineType());
         if (renderer.isInline() != renderer.parent()->childrenInline())
             childFlowStateChangesAndAffectsParentBlock(renderer);