Bug 93965

Created attachment 158323 [details] Ref test highlighting the problem

The second part of the issue stated here i.e.: * region.regionOverset returns 'fit' - while it should return 'overset' seems to have now been fixed. Verified on the latest nightly (windows) - r125742 The value for regionOverset now returned is 'overset' instead of 'fit'.

Created attachment 158814 [details] Patch

Comment on attachment 158814 [details] Patch Attachment 158814 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13515418 New failing tests: fast/repaint/region-painting-via-layout.html

Created attachment 158823 [details] Archive of layout-test-results from gce-cr-linux-04 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: gce-cr-linux-04 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid

The failing test case: fast/repaint/region-painting-via-layout.html, even without this fix, results in the same issue if position: relative is not specified. There seems to be a problem with repaint when the content overflows, and it happens even without the fix. For example, if I introduce a lag before the repaint, this issue doesn't occur i.e. the repaint is proper (even for the overflowing 20px). Can change the test case to have no overflow in region2 but that would also mean changing the expected results. Would really appreciate any help in this regard as am unsure on how to proceed with this issue.

*** Bug 92637 has been marked as a duplicate of this bug. ***

You do not need to have content be positioned to achieve the same result in the last region, it is enough to have the block that is collected into a flow thread have its own layer. For instance: <div id="text" style="-webkit-flow-into: flow; opacity: 0.9"> Text that overflows the region </div> <div class="region" style="-webkit-flow-from: flow; width: 150px; height: 100px;"></div> will clip the text at the region box, similar to uploaded test.

Comment on attachment 158814 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=158814&action=review Thanks for your patch! This is an informal review on the approach, so feel free to ignore my comments :) > Source/WebCore/rendering/RenderBox.cpp:3614 > + if ((child->hasSelfPaintingLayer() && !inRenderFlowThread()) || hasOverflowClip()) I would better change RenderRegion::overflowRectForFlowThreadPortion method to use RenderLayer::calculateLayerBounds instead. That one should know everything about the descendant layers and should compute the "right" overflow. I'm pretty sure that having your fix here, doesn't fix the overflow when you have a -webkit-transform: translate(1000px, 0px). Moreover, I suppose the visual overflow is not correctly calculated here when you have a transform that changes the coordinate system (ie. rotate).

Comment on attachment 158814 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=158814&action=review > Source/WebCore/ChangeLog:11 > + This comment is not correct and i would prefer to be changed. From the changelog comment it should be clear about what visual overflow you are talking, for instance the visual overflow of the elements that are collected inside a flow thread and have an attached layer. Also, the bug is not about positioned regions. > Source/WebCore/ChangeLog:19 > + Again, not the region is having the position: relative, but rather the content that is fragmented by the region. > Source/WebCore/ChangeLog:23 > + The child having itself a layer reproduces the problem, it does not need to be positioned.

Bug doesn't reproduce anymore. Adding the attached ref-tests to prevent regressions.

OK, actually the second part of the bug (the one with position: relative and overflow) *DOES* reproduce. Waiting for fix.

The bug regarding the case when the content as a CSS Transform (rotation) will be handled as bug 110198.

Comment on attachment 158814 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=158814&action=review >> Source/WebCore/rendering/RenderBox.cpp:3614 >> + if ((child->hasSelfPaintingLayer() && !inRenderFlowThread()) || hasOverflowClip()) > > I would better change RenderRegion::overflowRectForFlowThreadPortion method to use RenderLayer::calculateLayerBounds instead. That one should know everything about the descendant layers and should compute the "right" overflow. > > I'm pretty sure that having your fix here, doesn't fix the overflow when you have a -webkit-transform: translate(1000px, 0px). > Moreover, I suppose the visual overflow is not correctly calculated here when you have a transform that changes the coordinate system (ie. rotate). 1. calling RenderLayer from RenderRegion (a RenderBlock) would use outdated information (the RenderLayer tree is updated after the RenderObject tree). The change in the this patch fixes having position:relative and also opacity on content. I would go for pushing this patch in. 2. fixing the transform issue is now being tracked in bug 110198.

(In reply to comment #14) > (From update of attachment 158814 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=158814&action=review > > >> Source/WebCore/rendering/RenderBox.cpp:3614 > >> + if ((child->hasSelfPaintingLayer() && !inRenderFlowThread()) || hasOverflowClip()) > > > > I would better change RenderRegion::overflowRectForFlowThreadPortion method to use RenderLayer::calculateLayerBounds instead. That one should know everything about the descendant layers and should compute the "right" overflow. > > > > I'm pretty sure that having your fix here, doesn't fix the overflow when you have a -webkit-transform: translate(1000px, 0px). > > Moreover, I suppose the visual overflow is not correctly calculated here when you have a transform that changes the coordinate system (ie. rotate). > > 1. calling RenderLayer from RenderRegion (a RenderBlock) would use outdated information (the RenderLayer tree is updated after the RenderObject tree). > The change in the this patch fixes having position:relative and also opacity on content. > I would go for pushing this patch in. > > 2. fixing the transform issue is now being tracked in bug 110198. Thank-you for your comments Mihai. I shall try and give this patch another go.

Created attachment 191477 [details] Patch

Comment on attachment 191477 [details] Patch Attachment 191477 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17049138

Comment on attachment 191477 [details] Patch Attachment 191477 [details] did not pass cr-linux-debug-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17043101

Comment on attachment 191477 [details] Patch Attachment 191477 [details] did not pass qt-ews (qt): Output: http://webkit-commit-queue.appspot.com/results/17009105

Comment on attachment 191477 [details] Patch Attachment 191477 [details] did not pass qt-wk2-ews (qt): Output: http://webkit-commit-queue.appspot.com/results/17009106

Created attachment 191481 [details] Patch

Comment on attachment 191481 [details] Patch Attachment 191481 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17049159 New failing tests: fast/repaint/region-painting-via-layout.html

Created attachment 191693 [details] Patch

(In reply to comment #23) > Created an attachment (id=191693) [details] > Patch Maybe we should solve https://bugs.webkit.org/show_bug.cgi?id=76991 before this bug.

Comment on attachment 191693 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191693&action=review > Source/WebCore/rendering/RenderBox.cpp:4076 > - if (child->hasSelfPaintingLayer() || hasOverflowClip()) > + RenderFlowThread* flowThread = flowThreadContainingBlock(); > + if ((child->hasSelfPaintingLayer() && !flowThread) || hasOverflowClip()) This is not correct. You should never ever propagate visual overflow across a self-painting layer boundary.

Isn't regionOverset supposed to be about layoutOverflow and not visualOverflow? I'm confused as to why you're messing with visual overflow. Layout overflow does propagate across layers and accounts for transforms and relative positioning. Visual overflow is something different. It's just about shadows and outlines, etc.

This is finally fixed :). Closing!