[webkit-reviews] review canceled: [Bug 84487] [chromium] Add touch link highlight animation layers. : [Attachment 146898] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jun 11 14:00:38 PDT 2012


W. James MacLean <wjmaclean at chromium.org> has canceled W. James MacLean
<wjmaclean at chromium.org>'s request for review:
Bug 84487: [chromium] Add touch link highlight animation layers.
https://bugs.webkit.org/show_bug.cgi?id=84487

Attachment 146898: Patch
https://bugs.webkit.org/attachment.cgi?id=146898&action=review

------- Additional Comments from W. James MacLean <wjmaclean at chromium.org>
This patch attempts to resolve (in most cases) correct behaviour when iframes
and scrollable divs are used (wether or not they get their own layers).

It has the initial framework to make sure the highlight is clipped to the
boundaries of the scroll region, but that is delayed on a method for finding
the correct origin at which to place the clip layer when a non-composited
scroll region is used.

I've used an 'observer' to get relevant scroll events to the LinkHighlight
class, since highlights may now need to move as the region is scrolled
(depending on which layer is composited). I attached the observer to
ScrollAnimator as putting it in ScrollableArea seemed like a bad idea.

Is the use of an observer OK?

There is still work to be done, but I'd like to know if the overall approach is
reasonable before sorting out the remaining issues. The patch has gotten large
again, and needs to be broken up, so I'm not asking for a formal review but
rather a 'quick look' at the approach.

Still to be done:
1) get the correct origin for the clipLayer when a non-composited scroll region
is used, and enable clipping.
2) Make ScrollAnimator support a map of observers.
3) Determine logic for the case where nested scroll layers are used inside a
composited container.
4) Add unit tests for link highlights in frames.


More information about the webkit-reviews mailing list