[Webkit-unassigned] [Bug 69220] Fix FrameView::scrollElementToRect to take already scrolled amount into consideration

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Oct 3 12:55:01 PDT 2011


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





--- Comment #8 from Varun Jain <varunjain at chromium.org>  2011-10-03 12:55:01 PST ---
(In reply to comment #7)
> > The patch makes sure it is not relative to the current scroll amount (please see the updated layout test).
> 
> OK. Bug title could be improved - it says "take already scrolled amount into consideration", while you are making it so that the function actually ignores.
> 
> I agree with Fady that it's the use of scrollBy that seems wrong here.
> 
> > For context, please see https://bugs.webkit.org/show_bug.cgi?id=68198
> 
> There is no rationale in that bug. It's about moving some Chromium code into WebCore, but I don't see an explanation of why this code is needed in the first place. It's a pity that only Chromium developers looked at that bug.
> 
> A function that has the goal to make something visible should have "reveal" in its name to match the rest of WebCore. As opposed to scrolling to a point, a "reveal" function is allowed to make some platform specific decisions as to which side of the visible area the element will appear at, and how far from the side.

Hi Alexey,

The original CL (not the refactoring is this: https://bugs.webkit.org/show_bug.cgi?id=68192

The rational was to provide an API for the embedder to scroll an element on the page to a non-hidden area of the screen (note that this is not achievable by Element::scrollIntoViewIfNeeded since the view area may be partially obscured. In the original CL, I was instructed by the reviewer to move the scrolling code into webcore. There are valid reasons for this: 1. the WebKit API is not the place to add logic and should only act as an entry point, 2. Having it in WebCore makes it testable using window.internals. I had a discussion with the reviewer and we found that FrameView would be the best place to put this.

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



More information about the webkit-unassigned mailing list