Changeset 168286 in webkit

Timestamp:

May 5, 2014 7:47:33 AM (10 years ago)

Author:

abucur@adobe.com

Message:

[CSSRegions] Slider displayed wrong in regions
​https://bugs.webkit.org/show_bug.cgi?id=132243

Reviewed by Mihnea Ovidenie.

Source/WebCore:
In case a box didn't have a range, getRegionRangeForBox was searching for
the top-most unplittable ancestor. This is not correct in every case. It's
possible to have a box with range that has children without ranges (e.g. an
absolutely positioned inline box with shadow descendants).

I've modified getRegionRangeForBox to search for the first ancestor that
has a cached range and use that instead of looking for an unsplittable box.
The range of the box is the region at the top of the box, clamped by the
range of the ancestor. This will be correct all the time once all the layout
systems are region range aware and are able to cache it.

Test: fast/regions/positioned-slider-in-regions.html

• rendering/RenderFlowThread.cpp:

(WebCore::RenderFlowThread::hasRegionRangeForBox):
(WebCore::RenderFlowThread::getRegionRangeForBox):

• rendering/RenderFlowThread.h:

LayoutTests:
Tests that the thumb of positioned slider inside regions is correctly painted.

• fast/regions/positioned-slider-in-regions-expected.html: Added.
• fast/regions/positioned-slider-in-regions.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/regions/positioned-slider-in-regions-expected.html (added)
• LayoutTests/fast/regions/positioned-slider-in-regions.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderFlowThread.cpp (modified) (2 diffs)
• Source/WebCore/rendering/RenderFlowThread.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2014-05-05  Andrei Bucur  <abucur@adobe.com>
+
+        [CSSRegions] Slider displayed wrong in regions
+        https://bugs.webkit.org/show_bug.cgi?id=132243
+
+        Reviewed by Mihnea Ovidenie.
+
+        Tests that the thumb of positioned slider inside regions is correctly painted.
+
+        * fast/regions/positioned-slider-in-regions-expected.html: Added.
+        * fast/regions/positioned-slider-in-regions.html: Added.
+
 2014-05-05  Xabier Rodriguez Calvar  <calvaris@igalia.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2014-05-05  Andrei Bucur  <abucur@adobe.com>
+
+        [CSSRegions] Slider displayed wrong in regions
+        https://bugs.webkit.org/show_bug.cgi?id=132243
+
+        Reviewed by Mihnea Ovidenie.
+
+        In case a box didn't have a range, getRegionRangeForBox was searching for
+        the top-most unplittable ancestor. This is not correct in every case. It's
+        possible to have a box with range that has children without ranges (e.g. an
+        absolutely positioned inline box with shadow descendants).
+
+        I've modified getRegionRangeForBox to search for the first ancestor that
+        has a cached range and use that instead of looking for an unsplittable box.
+        The range of the box is the region at the top of the box, clamped by the
+        range of the ancestor. This will be correct all the time once all the layout
+        systems are region range aware and are able to cache it.
+
+        Test: fast/regions/positioned-slider-in-regions.html
+
+        * rendering/RenderFlowThread.cpp:
+        (WebCore::RenderFlowThread::hasRegionRangeForBox):
+        (WebCore::RenderFlowThread::getRegionRangeForBox):
+        * rendering/RenderFlowThread.h:
+
 2014-05-05  Andrei Bucur  <abucur@adobe.com>
 

trunk/Source/WebCore/rendering/RenderFlowThread.cpp

@@ -748 +748 @@
 }
 
+bool RenderFlowThread::hasRegionRangeForBox(const RenderBox* box) const
+{
+    ASSERT(box);
+
+    if (m_regionRangeMap.contains(box))
+        return true;
+
+    InlineElementBox* boxWrapper = box->inlineBoxWrapper();
+    if (boxWrapper && boxWrapper->root().containingRegion())
+        return true;
+
+    return false;
+}
+
 bool RenderFlowThread::getRegionRangeForBoxFromCachedInfo(const RenderBox* box, RenderRegion*& startRegion, RenderRegion*& endRegion) const
 {
@@ -787 +801 @@
     // If the unsplittable box has region range, then the start and end region for the box
     // should be equal with the region for the unsplittable box if any.
-    RenderBox* topMostUnsplittable = nullptr;
     RenderBox* cb = const_cast<RenderBox*>(box);
-    while (!cb->isRenderFlowThread()) {
-        if (cb->isUnsplittableForPagination())
-            topMostUnsplittable = cb;
+    RenderBox* cbToUse = nullptr;
+    while (!cb->isRenderFlowThread() && !cbToUse) {
+        // FIXME: Use the containingBlock() value once we patch all the layout systems to be region range aware
+        // (e.g. if we use containingBlock() the shadow controls of a video element won't get the range from the
+        // video box because it's not a block; they need to be patched separately).
         ASSERT(cb->parent());
         cb = cb->parent()->enclosingBox();
         ASSERT(cb);
-    }
-
-    if (topMostUnsplittable) {
-        if (getRegionRangeForBoxFromCachedInfo(topMostUnsplittable, startRegion, endRegion)) {
-            ASSERT(endRegion == startRegion);
-            return true;
-        }
+
+        if (hasRegionRangeForBox(cb))
+            cbToUse = cb;
+    }
+
+    // If a box doesn't have a cached region range it usually means the box belongs to a line so startRegion should be equal with endRegion.
+    // FIXME: Find the cases when this startRegion should not be equal with endRegion and make sure these boxes have cached region ranges.
+    if (cbToUse) {
+        startRegion = endRegion = const_cast<RenderFlowThread*>(this)->regionAtBlockOffset(cbToUse, box->offsetFromLogicalTopOfFirstPage(), true, DisallowRegionAutoGeneration);
+        return true;
     }
 

trunk/Source/WebCore/rendering/RenderFlowThread.h

@@ -142 +142 @@
     virtual void setRegionRangeForBox(const RenderBox*, RenderRegion*, RenderRegion*);
     bool getRegionRangeForBox(const RenderBox*, RenderRegion*& startRegion, RenderRegion*& endRegion) const;
-    bool hasRegionRangeForBox(const RenderBox* box) const { ASSERT(box); return m_regionRangeMap.contains(box); }
+    bool hasRegionRangeForBox(const RenderBox*) const;
 
     // Check if the object is in region and the region is part of this flow thread.