Summary: | [Chromium] Add a stub for WebView::getTouchHighlightQuads() | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Peter Beverloo <peter> | ||||||
Component: | WebKit API | Assignee: | Peter Beverloo <peter> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, dglazkov, fishd, jamesr, tkent+wkapi, trchen, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 66687, 79150, 91648 | ||||||||
Attachments: |
|
Description
Peter Beverloo
2012-08-02 08:59:57 PDT
Created attachment 156104 [details]
Patch
I'm not sure about the #if-guards as no others are used in WebView.h. Should I remove them? Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI. Comment on attachment 156104 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=156104&action=review > Source/Platform/chromium/public/WebTouchCandidatesInfo.h:48 > + WebTouchCandidatesInfo() > + : numberOfCandidates(0), > + : unitedBounds(), > + : smallestDimension(0) This isn't quite right. Try: WebTouchCandidatesInfo() : numberOfCandidates(0) , smallestDimension(0) > Source/WebKit/chromium/public/WebView.h:63 > +#if defined(OS_ANDROID) These ifdefs aren't needed. > Source/WebKit/chromium/public/WebView.h:474 > + // Touch ---------------------------------------------------------------- Two blank lines above section headings. > Source/WebKit/chromium/public/WebView.h:476 > +#if defined(OS_ANDROID) This #ifdef isn't needed. > Source/WebKit/chromium/public/WebView.h:483 > + virtual WebVector<WebFloatQuad> getTouchHighlightQuads(const WebPoint&, > + int padding, Should we use a WebRect instead? > Source/WebKit/chromium/public/WebView.h:485 > + WebTouchCandidatesInfo* outTouchInfo, > + WebColor& outTapHighlightColor) = 0; WebTouchCandidatesInfo* -> WebTouchCandidatesInfo& (we use non-const reference types for out parameters. Comment on attachment 156104 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=156104&action=review Thank you for the review! Could you take a look at the updated patch too, please? >> Source/Platform/chromium/public/WebTouchCandidatesInfo.h:48 >> + : smallestDimension(0) > > This isn't quite right. Try: > > WebTouchCandidatesInfo() > : numberOfCandidates(0) > , smallestDimension(0) Uch, bad one. Thanks. I've added an include to this file in WebViewImpl.h to make sure that it's being compiled. >> Source/WebKit/chromium/public/WebView.h:63 >> +#if defined(OS_ANDROID) > > These ifdefs aren't needed. Done. >> Source/WebKit/chromium/public/WebView.h:474 >> + // Touch ---------------------------------------------------------------- > > Two blank lines above section headings. Done. >> Source/WebKit/chromium/public/WebView.h:476 >> +#if defined(OS_ANDROID) > > This #ifdef isn't needed. Done. >> Source/WebKit/chromium/public/WebView.h:483 >> + int padding, > > Should we use a WebRect instead? I'd prefer not to change the API too much if it's likely to change as it is. Tien-Ren will have a much better idea than I do about what is necessary here. >> Source/WebKit/chromium/public/WebView.h:485 >> + WebColor& outTapHighlightColor) = 0; > > WebTouchCandidatesInfo* -> WebTouchCandidatesInfo& (we use non-const reference types for out parameters. Done. Created attachment 156109 [details]
Patch
Comment on attachment 156109 [details] Patch Clearing flags on attachment: 156109 Committed r124478: <http://trac.webkit.org/changeset/124478> All reviewed patches have been landed. Closing bug. |