RESOLVED FIXED 169991
[Cocoa] Add an option to exclude overflow when snapshotting a WKWebProcessPlugInNodeHandle
https://bugs.webkit.org/show_bug.cgi?id=169991
Summary [Cocoa] Add an option to exclude overflow when snapshotting a WKWebProcessPlu...
Andy Estes
Reported 2017-03-22 21:02:01 PDT
[Cocoa] Add an option to exclude overflow when snapshotting a WKWebProcessPlugInNodeHandle
Attachments
Patch (24.79 KB, patch)
2017-03-22 21:17 PDT, Andy Estes
no flags
Andy Estes
Comment 1 2017-03-22 21:02:50 PDT
Andy Estes
Comment 2 2017-03-22 21:17:13 PDT
Build Bot
Comment 3 2017-03-22 21:18:28 PDT
Attachment 305160 [details] did not pass style-queue: ERROR: Source/WebKit2/WebProcess/InjectedBundle/API/Cocoa/WKWebProcessPlugInNodeHandle.mm:29: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Tim Horton
Comment 4 2017-03-22 21:24:50 PDT
Comment on attachment 305160 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=305160&action=review > Source/WebKit2/WebProcess/InjectedBundle/API/Cocoa/WKWebProcessPlugInNodeHandle.mm:71 > - RefPtr<WebImage> image = _nodeHandle->renderedImage(options); > + RefPtr<WebImage> image = _nodeHandle->renderedImage(toSnapshotOptions(options), options & kWKSnapshotOptionsExcludeOverflow); Maybe it's obvious and I'm missing it, but why not add a SnapshotOptions bit for this? > Tools/TestWebKitAPI/Tests/WebKit2Cocoa/rendered-image-excluding-overflow.html:20 > + <div class=container> > + <div class=inner> Missing quotes, eww :)
Andy Estes
Comment 5 2017-03-22 22:17:50 PDT
(In reply to Tim Horton from comment #4) > Comment on attachment 305160 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=305160&action=review > > > Source/WebKit2/WebProcess/InjectedBundle/API/Cocoa/WKWebProcessPlugInNodeHandle.mm:71 > > - RefPtr<WebImage> image = _nodeHandle->renderedImage(options); > > + RefPtr<WebImage> image = _nodeHandle->renderedImage(toSnapshotOptions(options), options & kWKSnapshotOptionsExcludeOverflow); > > Maybe it's obvious and I'm missing it, but why not add a SnapshotOptions bit > for this? I didn't do this because SnapshotOptions is used in a bunch of other places (for drag images, text indicators, etc), but kWKSnapshotOptionsExcludeOverflow is only meaningful in this context. Thanks for the review!
WebKit Commit Bot
Comment 6 2017-03-22 23:21:29 PDT
Comment on attachment 305160 [details] Patch Clearing flags on attachment: 305160 Committed r214297: <http://trac.webkit.org/changeset/214297>
WebKit Commit Bot
Comment 7 2017-03-22 23:21:33 PDT
All reviewed patches have been landed. Closing bug.
Simon Fraser (smfr)
Comment 8 2017-03-23 12:11:10 PDT
Comment on attachment 305160 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=305160&action=review > Source/WebKit2/WebProcess/InjectedBundle/DOM/InjectedBundleNodeHandle.cpp:186 > + paintingRect = renderer->absoluteBoundingBoxRectIgnoringTransforms(); Did you test this on a transformed box?
Tim Horton
Comment 9 2017-03-23 12:15:54 PDT
Comment on attachment 305160 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=305160&action=review >> Source/WebKit2/WebProcess/InjectedBundle/DOM/InjectedBundleNodeHandle.cpp:186 >> + paintingRect = renderer->absoluteBoundingBoxRectIgnoringTransforms(); > > Did you test this on a transformed box? It surely doesn't work, but it's not a regression because paintingRootRect does the same thing.
Note You need to log in before you can comment on or make changes to this bug.