Bug 87569

Summary: [BlackBerry] Add a default tap highlight
Product: WebKit Reporter: Arvid Nilsson <anilsson>
Component: WebKit BlackBerryAssignee: Arvid Nilsson <anilsson>
Status: RESOLVED FIXED    
Severity: Normal CC: mifenton, rakuco, rwlbuis, tonikitoo, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 87566, 87567    
Bug Blocks:    
Attachments:
Description Flags
Patch none

Description Arvid Nilsson 2012-05-25 22:22:46 PDT
We used to require the embedder to implement tap highlight drawing.
            Now, a default tap highlight, implemented using the recently added
            accelerated compositing overlay layer support, can be used instead.
    
            The tap highlight zooms in when it appears, to provide visible feedback
            even though your finger obscures the whole link, and fades out when
            hidden. The appearance of the default tap highlight is not final.
    
            The default tap highlight can be overridden using the new
            WebPage::setTapHighlight() method.
    
            Reviewed by Mike Lattanzio and Mike Fenton.

Also

commit b4d678e9f2234041e2ccecea60968eca5ca0cf50
Author: Arvid Nilsson <anilsson@rim.com>
Date:   Tue May 8 17:47:14 2012 +0200

    2012-05-08  Arvid Nilsson  <anilsson@rim.com>
    
            Fix simulator build after AC change
    
            Reviewed by Commit Bot.
    
            * Api/WebPage_p.h:
Comment 1 Arvid Nilsson 2012-05-27 06:43:34 PDT
Created attachment 144231 [details]
Patch
Comment 2 Mike Fenton 2012-05-28 08:25:47 PDT
Reviewed internally, no new issues.
Comment 3 Rob Buis 2012-05-28 09:52:47 PDT
Comment on attachment 144231 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=144231&action=review

Looks good.

> Source/WebKit/blackberry/Api/WebTapHighlight.h:33
> +    virtual void draw(const Platform::IntRectRegion&, int red, int green, int blue, int alpha, bool hideAfterScroll) = 0;

Have you considered RGBA32?

> Source/WebKit/blackberry/WebKitSupport/DefaultTapHighlight.cpp:37
> +#include <SkCornerPathEffect.h>

Is this one needed?
Comment 4 Arvid Nilsson 2012-05-28 10:31:57 PDT
(In reply to comment #3)
> (From update of attachment 144231 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=144231&action=review
> 
> Looks good.
> 
> > Source/WebKit/blackberry/Api/WebTapHighlight.h:33
> > +    virtual void draw(const Platform::IntRectRegion&, int red, int green, int blue, int alpha, bool hideAfterScroll) = 0;
> 
> Have you considered RGBA32?
> 

In this case, it's public api, and there's no public color class or typedef afaik. The reason four inta are used, and not oe int, or 4 chars, is that the method signature is modelled after the old WebPageClient::drawTapHighlight() method. Or was it showTapHighlight?

> > Source/WebKit/blackberry/WebKitSupport/DefaultTapHighlight.cpp:37
> > +#include <SkCornerPathEffect.h>
> 
> Is this one needed?

Nope, this one is popping up like a jack in the box, I'll have to upload a new patch without it.
Comment 5 WebKit Review Bot 2012-05-28 11:54:13 PDT
Comment on attachment 144231 [details]
Patch

Clearing flags on attachment: 144231

Committed r118701: <http://trac.webkit.org/changeset/118701>
Comment 6 WebKit Review Bot 2012-05-28 11:54:19 PDT
All reviewed patches have been landed.  Closing bug.