Bug 118585

Summary: [WK2] Share Qt port's codes to find zoomable area with CoordinatedGraphics.
Product: WebKit Reporter: Eunmi Lee <enmi.lee>
Component: WebKit2Assignee: Eunmi Lee <enmi.lee>
Status: RESOLVED FIXED    
Severity: Normal CC: abecsi, cdumez, cmarcelo, commit-queue, gmak, gyuyoung.kim, lucas.de.marchi, luiz, menard, noam, rakuco, tonikitoo, zeno
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 118588    
Bug Blocks: 107631    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Eunmi Lee
Reported 2013-07-11 23:55:17 PDT
The codes to find zoomable area will be used in the coordinated graphics, so modify codes for Qt port to share with coordinated graphics.
Attachments
Patch (19.33 KB, patch)
2013-07-12 00:11 PDT, Eunmi Lee 		no flags
DetailsFormatted DiffDiff
Patch (28.49 KB, patch)
2013-07-12 05:57 PDT, Eunmi Lee 		no flags
DetailsFormatted DiffDiff
Patch (28.44 KB, patch)
2013-07-18 02:29 PDT, Eunmi Lee 		no flags
DetailsFormatted DiffDiff
Patch (28.40 KB, patch)
2013-07-18 05:02 PDT, Eunmi Lee 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Eunmi Lee
Comment 1 2013-07-12 00:11:08 PDT
Created attachment 206509 [details] Patch
Noam Rosenthal
Comment 2 2013-07-12 01:07:43 PDT
Comment on attachment 206509 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=206509&action=review > Source/WebKit2/UIProcess/WebPageProxy.cpp:2952 > +#if PLATFORM(QT) || USE(COORDINATED_GRAPHICS) No need for #if PLATFORM(QT) here Qt already uses coordinated graphics. > Source/WebKit2/UIProcess/WebPageProxy.cpp:2953 > +void WebPageProxy::findZoomableAreaForPoint(const IntPoint& point, const IntSize& area) Maybe we can put this in a new file (WebPageProxyCoordinatedGraphics)? > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:3221 > + IntRect zoomableArea = node->pixelSnappedBoundingBox(); This seems like a separate fix
Eunmi Lee
Comment 3 2013-07-12 01:16:42 PDT
Comment on attachment 206509 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=206509&action=review Thank you for comments :) >> Source/WebKit2/UIProcess/WebPageProxy.cpp:2952 >> +#if PLATFORM(QT) || USE(COORDINATED_GRAPHICS) > > No need for #if PLATFORM(QT) here > Qt already uses coordinated graphics. That's good, I will modify all "PLATFORM(QT) || USE(COORDINATED_GRAPHICS)". >> Source/WebKit2/UIProcess/WebPageProxy.cpp:2953 >> +void WebPageProxy::findZoomableAreaForPoint(const IntPoint& point, const IntSize& area) > > Maybe we can put this in a new file (WebPageProxyCoordinatedGraphics)? OK. I will add WebPageProxyCoordinatedGraphics.cpp. >> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:3221 >> + IntRect zoomableArea = node->pixelSnappedBoundingBox(); > > This seems like a separate fix Right. I will make separate patch for that.
Eunmi Lee
Comment 4 2013-07-12 05:57:03 PDT
Created attachment 206529 [details] Patch
Noam Rosenthal
Comment 5 2013-07-17 19:21:07 PDT
Comment on attachment 206529 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=206529&action=review I'm ok with the change in general, but the Changelog needs to be rewritten and it needs owner approval since it touches WebPage.*. > Source/WebKit2/ChangeLog:13 > + The codes to find zoomable area will be used in the coordinated graphics, > + so modify codes to be able to be used not only in the Qt port but also > + coordinated graphics. > + > + Additionally, Add the WKViewFindZoomableAreaForRect API and didFindZoomableArea > + callback for coordinated graphics and they will be used in the EFL port. Please use correct English.
Eunmi Lee
Comment 6 2013-07-18 01:47:11 PDT
Comment on attachment 206529 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=206529&action=review >> Source/WebKit2/ChangeLog:13 >> + callback for coordinated graphics and they will be used in the EFL port. > > Please use correct English. OK, I will re-write the changelog.
Eunmi Lee
Comment 7 2013-07-18 02:29:36 PDT
Created attachment 206972 [details] Patch Update changelog.
Noam Rosenthal
Comment 8 2013-07-18 02:38:18 PDT
Comment on attachment 206972 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=206972&action=review I'm ok with this, but it touches PageClient.h so please ask for WK2 owner review. > Source/WebKit2/ChangeLog:11 > + to use that fuction, so extend codes for Qt port to be usable in the > + CoordinatedGraphics and add API and callback to use that. extend codes for Qt port -> extract the code from the Qt port
Eunmi Lee
Comment 9 2013-07-18 04:52:57 PDT
OK, thanks for review! (In reply to comment #8) > (From update of attachment 206972 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=206972&action=review > > I'm ok with this, but it touches PageClient.h so please ask for WK2 owner review. > > > Source/WebKit2/ChangeLog:11 > > + to use that fuction, so extend codes for Qt port to be usable in the > > + CoordinatedGraphics and add API and callback to use that. > > extend codes for Qt port -> extract the code from the Qt port
Eunmi Lee
Comment 10 2013-07-18 05:02:23 PDT
Created attachment 206982 [details] Patch Update changelog.
Noam Rosenthal
Comment 11 2013-07-18 05:08:36 PDT
Comment on attachment 206982 [details] Patch LGTM, requires owner approval.
WebKit Commit Bot
Comment 12 2013-07-18 19:10:54 PDT
Comment on attachment 206982 [details] Patch Clearing flags on attachment: 206982 Committed r152878: <http://trac.webkit.org/changeset/152878>
WebKit Commit Bot
Comment 13 2013-07-18 19:10:59 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.