Bug 87569 - [BlackBerry] Add a default tap highlight
Summary: [BlackBerry] Add a default tap highlight
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit BlackBerry (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Arvid Nilsson
URL:
Keywords:
Depends on: 87566 87567
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-25 22:22 PDT by Arvid Nilsson
Modified: 2012-05-28 11:54 PDT (History)
5 users (show)

See Also:


Attachments
Patch (19.98 KB, patch)
2012-05-27 06:43 PDT, Arvid Nilsson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.