[Webkit-unassigned] [Bug 83981] [Qt][WK2] Fixed elements position is wrong after zooming.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=83981


Kenneth Rohde Christiansen <kenneth at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #137261|review?                     |review+
               Flag|                            |




--- Comment #2 from Kenneth Rohde Christiansen <kenneth at webkit.org>  2012-04-16 04:00:16 PST ---
(From update of attachment 137261)
View in context: https://bugs.webkit.org/attachment.cgi?id=137261&action=review

> ManualTests/remove-add-fixed-position.html:5
> +.d1{position:fixed;top:5;right:5;z-index:2;overflow:hidden;}
> +.o {background:green;height:40px;width:200px;}

Some spacing here would be nice :-) or even some newlines.

> Source/WebCore/ChangeLog:8
> +        When setFixedVisibleContentRect is called, we mark all fixed elements in the frame, for layout.

I don't get the commas here :-)

> Source/WebCore/ChangeLog:11
> +        They are added and removed at the same time that they are added and removed from their parent RenderBlock.
> +        The idea is taken from the iOS5.1 branch, at opensource.apple.com.

So you didn't reuse code? Based on opensource code from the iOS port. ?

> Source/WebCore/page/FrameView.cpp:1704
>  {
> +    if (visibleContentRect.size() != this->fixedVisibleContentRect().size()) {

Maybe this deserves a little comment: 

// When the viewport size changes or the content is scaled, we need to
// reposition the fixed positioned elements.

as it is not so obvious that this is called when we scale the content

> Source/WebCore/rendering/RenderBlock.cpp:3423
> +    if (o->style()->position() == FixedPosition) {
> +        if (view())

why not merge those two if's ? if (view() && o->style()->position() == FixedPosition)

> Source/WebCore/rendering/RenderBlock.cpp:3434
> +    if (view())
> +        view()->removeFixedPositionedObject(o);

So this will fail in most cases right.

> Source/WebCore/rendering/RenderView.cpp:923
> +void RenderView::setFixedPositionedObjectsNeedLayout()

need*S* no?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.