WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Andy Estes
Comment 1
2017-03-22 21:02:50 PDT
rdar://problem/30141083
Andy Estes
Comment 2
2017-03-22 21:17:13 PDT
Created
attachment 305160
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug