Add isContinuation bit
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 Oct 2017 10:38:37 +0000 (10:38 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 10 Oct 2017 10:38:37 +0000 (10:38 +0000)
https://bugs.webkit.org/show_bug.cgi?id=178084

Reviewed by Zalan Bujtas.

Currently continuations are identified indirectly by comparing renderer pointer with the element renderer pointer.
This is bug prone and fails to cover anonymous continuations.

* accessibility/AccessibilityRenderObject.cpp:
(WebCore::firstChildConsideringContinuation):
(WebCore::startOfContinuations):
(WebCore::firstChildIsInlineContinuation):
(WebCore::AccessibilityRenderObject::computeAccessibilityIsIgnored const):

    Ignore first-letter fragment. This worked before because first-letter renderers
    were mistakenly considered inline element continuations (see below).

* rendering/RenderBoxModelObject.cpp:
(WebCore::RenderBoxModelObject::setContinuation):
* rendering/RenderElement.cpp:
(WebCore::RenderElement::RenderElement):
* rendering/RenderElement.h:
(WebCore::RenderElement::hasContinuation const):
(WebCore::RenderElement::isContinuation const):
(WebCore::RenderElement::setIsContinuation):

    The new bit.

(WebCore::RenderElement::isElementContinuation const):
(WebCore::RenderElement::isInlineElementContinuation const):
* rendering/RenderInline.cpp:
(WebCore::RenderInline::addChildIgnoringContinuation):
(WebCore::RenderInline::cloneAsContinuation const):
(WebCore::RenderInline::splitInlines):
(WebCore::RenderInline::childBecameNonInline):
(WebCore::RenderInline::clone const): Deleted.
* rendering/RenderInline.h:
* rendering/RenderObject.h:
(WebCore::RenderObject::isAnonymousBlock const):
(WebCore::RenderObject::isElementContinuation const): Deleted.

    The old continuation test was 'node() && node()->renderer() != this'
    This was fragile as nulling the renderer will make it fail.
    It was also wrong for first-letter renderers (isElementContinuation was true for them).

(WebCore::RenderObject::isInlineElementContinuation const): Deleted.

    Move to RenderElement.

(WebCore::RenderObject::isBlockElementContinuation const): Deleted.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@223127 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/accessibility/AccessibilityRenderObject.cpp
Source/WebCore/rendering/RenderBoxModelObject.cpp
Source/WebCore/rendering/RenderElement.cpp
Source/WebCore/rendering/RenderElement.h
Source/WebCore/rendering/RenderInline.cpp
Source/WebCore/rendering/RenderInline.h
Source/WebCore/rendering/RenderObject.h

index 33d06e3..102a85e 100644 (file)
@@ -1,3 +1,56 @@
+2017-10-09  Antti Koivisto  <antti@apple.com>
+
+        Add isContinuation bit
+        https://bugs.webkit.org/show_bug.cgi?id=178084
+
+        Reviewed by Zalan Bujtas.
+
+        Currently continuations are identified indirectly by comparing renderer pointer with the element renderer pointer.
+        This is bug prone and fails to cover anonymous continuations.
+
+        * accessibility/AccessibilityRenderObject.cpp:
+        (WebCore::firstChildConsideringContinuation):
+        (WebCore::startOfContinuations):
+        (WebCore::firstChildIsInlineContinuation):
+        (WebCore::AccessibilityRenderObject::computeAccessibilityIsIgnored const):
+
+            Ignore first-letter fragment. This worked before because first-letter renderers
+            were mistakenly considered inline element continuations (see below).
+
+        * rendering/RenderBoxModelObject.cpp:
+        (WebCore::RenderBoxModelObject::setContinuation):
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::RenderElement):
+        * rendering/RenderElement.h:
+        (WebCore::RenderElement::hasContinuation const):
+        (WebCore::RenderElement::isContinuation const):
+        (WebCore::RenderElement::setIsContinuation):
+
+            The new bit.
+
+        (WebCore::RenderElement::isElementContinuation const):
+        (WebCore::RenderElement::isInlineElementContinuation const):
+        * rendering/RenderInline.cpp:
+        (WebCore::RenderInline::addChildIgnoringContinuation):
+        (WebCore::RenderInline::cloneAsContinuation const):
+        (WebCore::RenderInline::splitInlines):
+        (WebCore::RenderInline::childBecameNonInline):
+        (WebCore::RenderInline::clone const): Deleted.
+        * rendering/RenderInline.h:
+        * rendering/RenderObject.h:
+        (WebCore::RenderObject::isAnonymousBlock const):
+        (WebCore::RenderObject::isElementContinuation const): Deleted.
+
+            The old continuation test was 'node() && node()->renderer() != this'
+            This was fragile as nulling the renderer will make it fail.
+            It was also wrong for first-letter renderers (isElementContinuation was true for them).
+
+        (WebCore::RenderObject::isInlineElementContinuation const): Deleted.
+
+            Move to RenderElement.
+
+        (WebCore::RenderObject::isBlockElementContinuation const): Deleted.
+
 2017-10-10  Joanmarie Diggs  <jdiggs@igalia.com>
 
         AX: [ATK] STATE_CHECKABLE should be removed from radio buttons in radiogroups with aria-readonly="true"
index e4155d1..3a23997 100644 (file)
@@ -183,7 +183,7 @@ static inline RenderObject* firstChildConsideringContinuation(RenderObject& rend
     // We don't want to include the end of a continuation as the firstChild of the
     // anonymous parent, because everything has already been linked up via continuation.
     // CSS first-letter selector is an example of this case.
-    if (renderer.isAnonymous() && firstChild && firstChild->isInlineElementContinuation())
+    if (renderer.isAnonymous() && is<RenderElement>(firstChild) && downcast<RenderElement>(*firstChild).isInlineElementContinuation())
         firstChild = nullptr;
     
     if (!firstChild && isInlineWithContinuation(renderer))
@@ -248,12 +248,15 @@ AccessibilityObject* AccessibilityRenderObject::lastChild() const
 
 static inline RenderInline* startOfContinuations(RenderObject& renderer)
 {
-    if (renderer.isInlineElementContinuation() && is<RenderInline>(renderer.node()->renderer()))
+    if (!is<RenderElement>(renderer))
+        return nullptr;
+    auto& renderElement = downcast<RenderElement>(renderer);
+    if (renderElement.isInlineElementContinuation() && is<RenderInline>(renderElement.element()->renderer()))
         return downcast<RenderInline>(renderer.node()->renderer());
 
     // Blocks with a previous continuation always have a next continuation
-    if (is<RenderBlock>(renderer) && downcast<RenderBlock>(renderer).inlineElementContinuation())
-        return downcast<RenderInline>(downcast<RenderBlock>(renderer).inlineElementContinuation()->element()->renderer());
+    if (is<RenderBlock>(renderElement) && downcast<RenderBlock>(renderElement).inlineElementContinuation())
+        return downcast<RenderInline>(downcast<RenderBlock>(renderElement).inlineElementContinuation()->element()->renderer());
 
     return nullptr;
 }
@@ -307,7 +310,7 @@ static inline RenderObject* childBeforeConsideringContinuations(RenderInline* re
 static inline bool firstChildIsInlineContinuation(RenderElement& renderer)
 {
     RenderObject* child = renderer.firstChild();
-    return child && child->isInlineElementContinuation();
+    return is<RenderElement>(child) && downcast<RenderElement>(*child).isInlineElementContinuation();
 }
 
 AccessibilityObject* AccessibilityRenderObject::previousSibling() const
@@ -1246,6 +1249,8 @@ bool AccessibilityRenderObject::computeAccessibilityIsIgnored() const
                 return true;
             if (altTextInclusion == IncludeObject)
                 return false;
+            if (downcast<RenderTextFragment>(renderText).firstLetter())
+                return true;
         }
 
         // text elements that are just empty whitespace should not be returned
index eb2dd3d..3280bf7 100644 (file)
@@ -2454,6 +2454,7 @@ RenderBoxModelObject* RenderBoxModelObject::continuation() const
 
 void RenderBoxModelObject::setContinuation(RenderBoxModelObject* continuation)
 {
+    ASSERT(!continuation || continuation->isContinuation());
     if (continuation)
         continuationMap().set(this, makeWeakPtr(continuation));
     else if (hasContinuation())
index 41779a0..9e0cd93 100644 (file)
@@ -105,6 +105,7 @@ inline RenderElement::RenderElement(ContainerNode& elementOrDocument, RenderStyl
     , m_hasPausedImageAnimations(false)
     , m_hasCounterNodeMap(false)
     , m_hasContinuation(false)
+    , m_isContinuation(false)
     , m_hasValidCachedFirstLineStyle(false)
     , m_renderBlockHasMarginBeforeQuirk(false)
     , m_renderBlockHasMarginAfterQuirk(false)
index b294ddf..06173b1 100644 (file)
@@ -204,7 +204,6 @@ public:
     void drawLineForBoxSide(GraphicsContext&, const FloatRect&, BoxSide, Color, EBorderStyle, float adjacentWidth1, float adjacentWidth2, bool antialias = false) const;
 
     bool childRequiresTable(const RenderObject& child) const;
-    bool hasContinuation() const { return m_hasContinuation; }
 
 #if ENABLE(TEXT_AUTOSIZING)
     void adjustComputedFontSizesOnBlocks(float size, float visibleWidth);
@@ -222,6 +221,12 @@ public:
     // the child.
     virtual void updateAnonymousChildStyle(const RenderObject&, RenderStyle&) const { };
 
+    bool hasContinuation() const { return m_hasContinuation; }
+    bool isContinuation() const { return m_isContinuation; }
+    void setIsContinuation() { m_isContinuation = true; }
+    bool isElementContinuation() const { return isContinuation() && !isAnonymous(); }
+    bool isInlineElementContinuation() const { return isElementContinuation() && isInline(); }
+
 protected:
     enum BaseTypeFlag {
         RenderLayerModelObjectFlag  = 1 << 0,
@@ -332,6 +337,7 @@ private:
     unsigned m_hasPausedImageAnimations : 1;
     unsigned m_hasCounterNodeMap : 1;
     unsigned m_hasContinuation : 1;
+    unsigned m_isContinuation : 1;
     mutable unsigned m_hasValidCachedFirstLineStyle : 1;
 
     unsigned m_renderBlockHasMarginBeforeQuirk : 1;
index b054ca4..82039c6 100644 (file)
@@ -342,6 +342,7 @@ void RenderInline::addChildIgnoringContinuation(RenderPtr<RenderObject> newChild
 
         auto newBox = createRenderer<RenderBlockFlow>(document(), WTFMove(newStyle));
         newBox->initializeStyle();
+        newBox->setIsContinuation();
         RenderBoxModelObject* oldContinuation = continuation();
         setContinuation(newBox.get());
 
@@ -354,12 +355,13 @@ void RenderInline::addChildIgnoringContinuation(RenderPtr<RenderObject> newChild
     child.setNeedsLayoutAndPrefWidthsRecalc();
 }
 
-RenderPtr<RenderInline> RenderInline::clone() const
+RenderPtr<RenderInline> RenderInline::cloneAsContinuation() const
 {
     RenderPtr<RenderInline> cloneInline = createRenderer<RenderInline>(*element(), RenderStyle::clone(style()));
     cloneInline->initializeStyle();
     cloneInline->setFragmentedFlowState(fragmentedFlowState());
     cloneInline->setHasOutlineAutoAncestor(hasOutlineAutoAncestor());
+    cloneInline->setIsContinuation();
     return cloneInline;
 }
 
@@ -368,7 +370,7 @@ void RenderInline::splitInlines(RenderBlock* fromBlock, RenderBlock* toBlock,
                                 RenderObject* beforeChild, RenderBoxModelObject* oldCont)
 {
     // Create a clone of this inline.
-    RenderPtr<RenderInline> cloneInline = clone();
+    RenderPtr<RenderInline> cloneInline = cloneAsContinuation();
 #if ENABLE(FULLSCREEN_API)
     // If we're splitting the inline containing the fullscreened element,
     // |beforeChild| may be the renderer for the fullscreened element. However,
@@ -435,7 +437,7 @@ void RenderInline::splitInlines(RenderBlock* fromBlock, RenderBlock* toBlock,
         if (splitDepth < cMaxSplitDepth) {
             // Create a new clone.
             RenderPtr<RenderInline> cloneChild = WTFMove(cloneInline);
-            cloneInline = downcast<RenderInline>(*current).clone();
+            cloneInline = downcast<RenderInline>(*current).cloneAsContinuation();
 
             // Insert our child clone as the first child.
             cloneInline->addChildIgnoringContinuation(WTFMove(cloneChild));
@@ -1375,6 +1377,7 @@ void RenderInline::childBecameNonInline(RenderElement& child)
 {
     // We have to split the parent flow.
     auto newBox = containingBlock()->createAnonymousBlock();
+    newBox->setIsContinuation();
     RenderBoxModelObject* oldContinuation = continuation();
     setContinuation(newBox.get());
     RenderObject* beforeChild = child.nextSibling();
index 30f96dd..5137aa5 100644 (file)
@@ -167,7 +167,7 @@ private:
     void addAnnotatedRegions(Vector<AnnotatedRegionValue>&) final;
 #endif
     
-    RenderPtr<RenderInline> clone() const;
+    RenderPtr<RenderInline> cloneAsContinuation() const;
 
     void paintOutlineForLine(GraphicsContext&, const LayoutPoint&, const LayoutRect& prevLine, const LayoutRect& thisLine, const LayoutRect& nextLine, const Color&);
     RenderBoxModelObject* continuationBefore(RenderObject* beforeChild);
index 2572885..ffe5e56 100644 (file)
@@ -411,9 +411,6 @@ public:
 #endif
             ;
     }
-    bool isElementContinuation() const { return node() && node()->renderer() != this; }
-    bool isInlineElementContinuation() const { return isElementContinuation() && isInline(); }
-    bool isBlockElementContinuation() const { return isElementContinuation() && !isInline(); }
 
     bool isFloating() const { return m_bitfields.floating(); }