Bug 103452

fixed layer fount, but do we have backing? (nil) fixed layer fount, but do we have backing? 0x7fd4e2768380 In static void updateOffsetFromViewportForSelf(RenderLayer* renderLayer) the fixed elements fail because they have no backing.

This is the check which fails in 1951 bool RenderLayerCompositor::requiresCompositingForPosition(RenderObject* renderer, const RenderLayer* layer) const 1977 // Fixed position elements that are invisible in the current view don't get their own layer. 1978 if (FrameView* frameView = m_renderView->frameView()) { 1979 IntRect viewBounds = IntRect(IntPoint(frameView->scrollOffsetForFixedPosition()), frameView->layoutSize()); 1980 IntRect layerBounds = calculateCompositedBounds(layer, rootRenderLayer()); 1981 if (!viewBounds.intersects(layerBounds)) 1982 return false; 1983 }

This fixes it, but I don't know why things were done the other way: --- a/Source/WebCore/rendering/RenderLayerCompositor.cpp +++ b/Source/WebCore/rendering/RenderLayerCompositor.cpp @@ -1974,7 +1974,8 @@ bool RenderLayerCompositor::requiresCompositingForPosition(RenderObject* rendere // Fixed position elements that are invisible in the current view don't get their own layer. if (FrameView* frameView = m_renderView->frameView()) { - IntRect viewBounds = IntRect(IntPoint(frameView->scrollOffsetForFixedPosition()), frameView->layoutSize()); + //IntRect viewBounds = IntRect(IntPoint(frameView->scrollOffsetForFixedPosition()), frameView->layoutSize()); + IntRect viewBounds = frameView->visibleContentRect(); IntRect layerBounds = calculateCompositedBounds(layer, rootRenderLayer()); if (!viewBounds.intersects(layerBounds)) return false;

Qt is affected by this as well.

Created attachment 177240 [details] Patch

What's the difference between visibleContentRect().location() and scrollOffsetForFixedPosition()?

(In reply to comment #6) > What's the difference between visibleContentRect().location() and scrollOffsetForFixedPosition()? I am not really sure, so it would be good with Beth's comments here. I think that is relates t how you are doing scrolling with the scrolling coordinator which we don't use. The method actually calls a similar name method in ScrollingCoordinator. Maybe we could ifdef it for platforms not using ScrollingCoordinator. But I would like to first understand if there is something that I am missing here.

(In reply to comment #6) > What's the difference between visibleContentRect().location() and scrollOffsetForFixedPosition()? scrollOffsetForFixedPosition() is adjusted for scroll origin, frame scale factor, and he Settings::fixedElementsLayoutRelativeToFrame(). I'm not sure why the code wouldn't work on other platforms. In case it's not clear from the svn-blaming, I did not write that code, I just moved it so that it could also be called from the scrolling thread.

Comment on attachment 177240 [details] Patch Please survey the code for other places which might get this wrong. Maybe scrollOffsetForFixedPosition() needs some comments next to it in the header.

(In reply to comment #8) > (In reply to comment #6) > > What's the difference between visibleContentRect().location() and scrollOffsetForFixedPosition()? > > scrollOffsetForFixedPosition() is adjusted for scroll origin, frame scale factor, and he Settings::fixedElementsLayoutRelativeToFrame(). I'm not sure why the code wouldn't work on other platforms. In case it's not clear from the svn-blaming, I did not write that code, I just moved it so that it could also be called from the scrolling thread. For one, Qt and EFL doesn't use frameScaleFactor() but does scaling outside of WebCore. We might consider using it, if there are compelling reasons for doing so, beyond sharing more code paths.

I don't think we want this change on the Mac!

(In reply to comment #11) > I don't think we want this change on the Mac! Ok, I can ifdef it then. Can you explain me in that situations IntRect(IntPoint(frameView->scrollOffsetForFixedPosition()), frameView->layoutSize()); is different that the visible contents rect on mac? Maybe there are some cases we need to be aware of as well.

(In reply to comment #12) > (In reply to comment #11) > > I don't think we want this change on the Mac! > > Ok, I can ifdef it then. Can you explain me in that situations IntRect(IntPoint(frameView->scrollOffsetForFixedPosition()), frameView->layoutSize()); is different that the visible contents rect on mac? > > Maybe there are some cases we need to be aware of as well. I take it back! scrollOffsetForFixedPosition() is needed sometimes in the Mac code because visibleContentRect() will return negative offsets when you are in the rubber-band phase of a scroll on the Mac. However, we don't need to worry about that in the RenderLayerCompositor code, so I think it is safe to use visibleContentRect here. Sorry for the confusion!

> I take it back! scrollOffsetForFixedPosition() is needed sometimes in the Mac code because visibleContentRect() will return negative offsets when you are in the rubber-band phase of a scroll on the Mac. However, we don't need to worry about that in the RenderLayerCompositor code, so I think it is safe to use visibleContentRect here. > > Sorry for the confusion! OK, thanks for the explanation! I will land it then.

Comment on attachment 177240 [details] Patch Rejecting attachment 177240 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: ripts/update-webkit line 152. Failed to run "['Tools/Scripts/update-webkit', '--chromium', '--force-update']" exit_code: 2 Updating OpenSource From http://git.chromium.org/external/Webkit ae22de8..2a054f9 HEAD -> origin/HEAD error: Ref refs/remotes/origin/master is at 2a054f9cb1bc438521d4764c001d072db3d434b7 but expected ae22de8513848241262c5d0b497139a3847cd99d ! ae22de8..2a054f9 master -> origin/master (unable to update local ref) Died at Tools/Scripts/update-webkit line 152. Full output: http://queues.webkit.org/results/15132151

Landed in 136452

This change seems to be causing some trouble when page pageScaleFactor != 1. This is because viewBounds is now in logical pixels while layerBounds is in CSS pixels. I'll put together a patch.

(In reply to comment #17) > This change seems to be causing some trouble when page pageScaleFactor != 1. This is because viewBounds is now in logical pixels while layerBounds is in CSS pixels. I'll put together a patch. Ah sorry about that. For Qt and EFL the viewport contents rect is in css pixels as we do scaling outside of webcore (not using frameScaleFactor)

FYI, Xianzhu has a fix here: https://bugs.webkit.org/show_bug.cgi?id=104303.