[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





--- Comment #3 from Yael <yael.aharon at nokia.com>  2012-04-16 06:17:33 PST ---
(In reply to comment #2)
Thanks for the review :)
> (From update of attachment 137261 [details])
> 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.
> 
ok
> > 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 :-)
> 
removed :)
> > 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. ?
> 
Not everything is there.I needed to fill the blanks.
> > 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
> 
ok
> > 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)
> 
ok
> > 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?
objects need, not needs :)

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