Skip to content

Commit

Permalink
Merge r230088 - Web Automation: clipToViewport is ignored for element…
Browse files Browse the repository at this point in the history
… screenshots

https://bugs.webkit.org/show_bug.cgi?id=184158
<rdar://problem/39014307>

Reviewed by Timothy Hatcher.

In §19.2 Take Element Screenshot, step 5.2 says that we should clip
the element screenshot rect with the visible viewport rect. We don't
do that right now even though we pass over clipToViewport.

* WebProcess/Automation/WebAutomationSessionProxy.cpp:
(WebKit::snapshotRectForScreenshot):
Clip the rect to viewport if needed.

(WebKit::WebAutomationSessionProxy::takeScreenshot):
This scrollIntoView is misplaced; by this point we have already done
the math to figure out the screenshot rect. Move it before computing the rect.
  • Loading branch information
burg authored and carlosgcampos committed Apr 9, 2018
1 parent 8e8df7a commit cea5df2
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 4 deletions.
20 changes: 20 additions & 0 deletions Source/WebKit/ChangeLog
@@ -1,3 +1,23 @@
2018-03-29 Brian Burg <bburg@apple.com>

Web Automation: clipToViewport is ignored for element screenshots
https://bugs.webkit.org/show_bug.cgi?id=184158
<rdar://problem/39014307>

Reviewed by Timothy Hatcher.

In §19.2 Take Element Screenshot, step 5.2 says that we should clip
the element screenshot rect with the visible viewport rect. We don't
do that right now even though we pass over clipToViewport.

* WebProcess/Automation/WebAutomationSessionProxy.cpp:
(WebKit::snapshotRectForScreenshot):
Clip the rect to viewport if needed.

(WebKit::WebAutomationSessionProxy::takeScreenshot):
This scrollIntoView is misplaced; by this point we have already done
the math to figure out the screenshot rect. Move it before computing the rect.

2018-03-29 Carlos Eduardo Ramalho <cadubentzen@gmail.com>

[WPE] Floating point exception in WebEventFactory::createWebWheelEvent
Expand Down
16 changes: 12 additions & 4 deletions Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp
Expand Up @@ -663,12 +663,20 @@ void WebAutomationSessionProxy::selectOptionElement(uint64_t pageID, uint64_t fr

static WebCore::IntRect snapshotRectForScreenshot(WebPage& page, WebCore::Element* element, bool clipToViewport)
{
auto* frameView = page.mainFrameView();
if (!frameView)
return { };

if (element) {
if (!element->renderer())
return { };

WebCore::LayoutRect topLevelRect;
return WebCore::snappedIntRect(element->renderer()->paintingRootRect(topLevelRect));
WebCore::IntRect elementRect = WebCore::snappedIntRect(element->renderer()->paintingRootRect(topLevelRect));
if (clipToViewport)
elementRect.intersect(frameView->visibleContentRect());

return elementRect;
}

if (auto* frameView = page.mainFrameView())
Expand Down Expand Up @@ -705,16 +713,16 @@ void WebAutomationSessionProxy::takeScreenshot(uint64_t pageID, uint64_t frameID
}
}

if (coreElement && scrollIntoViewIfNeeded)
coreElement->scrollIntoViewIfNeeded(false);

String screenshotErrorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::ScreenshotError);
WebCore::IntRect snapshotRect = snapshotRectForScreenshot(*page, coreElement, clipToViewport);
if (snapshotRect.isEmpty()) {
WebProcess::singleton().parentProcessConnection()->send(Messages::WebAutomationSession::DidTakeScreenshot(callbackID, handle, screenshotErrorType), 0);
return;
}

if (coreElement && scrollIntoViewIfNeeded)
coreElement->scrollIntoViewIfNeeded(false);

RefPtr<WebImage> image = page->scaledSnapshotWithOptions(snapshotRect, 1, SnapshotOptionsShareable);
if (!image) {
WebProcess::singleton().parentProcessConnection()->send(Messages::WebAutomationSession::DidTakeScreenshot(callbackID, handle, screenshotErrorType), 0);
Expand Down

0 comments on commit cea5df2

Please sign in to comment.