[Screenshotting] Add captureBeyondViewport: false to workaround a res… #131877

tsullivan · 2022-05-09T22:32:59Z

Summary

captureBeyondViewport flag

This adds captureBeyondViewport: false to our call of Puppeteer's Page.screenshot function to work around a bug that triggers an implicit resize inside of Puppeteer when we call that method.

From the Puppeteer docs:

captureBeyondViewport When true, captures screenshot beyond the viewport. When false, falls back to old behaviour, and cuts the screenshot by the viewport size. Defaults to true.

Adding this flag is high risk, because any part of the page that isn't contained in the viewport will be invisible. This can lead to screenshots that are cut-off or blank. This PR avoids that problem by doing a final resize of the viewport, to ensure that it includes the element to capture, before the screenshot is captured.

Checklist

Test Reporting developer example plugin
Test Canvas PDF: Default layout
Test Canvas PDF: "full page" layout
Test Dashboard PNG/PDF
Test Dashboard PDF Print Layout
Test Agg-based visualization from the editor
Test TSVB visualization from the editor
Unit or functional tests were updated or added to match the most common scenarios

Release Note

Fixed an issue in PDF/PNG reporting where visualization panels could sometimes appear blank.

…ize bug in Puppeteer

commit 9e5c688aefb9ff725a38b3c43402dfb96550a96a Author: Timothy Sullivan <tsullivan@elastic.co> Date: Fri May 6 14:56:11 2022 -0700 get real commit c05056e69145a1ad2e95f112b3987f2e88ba9408 Merge: 846ae81 8aa9241 Author: Timothy Sullivan <tsullivan@elastic.co> Date: Fri May 6 14:15:36 2022 -0700 Merge remote-tracking branch 'elastic/main' into reporting/ks-2573 commit 846ae81 Merge: 64b4c33 99c659c Author: Timothy Sullivan <tsullivan@elastic.co> Date: Wed May 4 19:28:17 2022 -0700 Merge remote-tracking branch 'elastic/main' into reporting/ks-2573 commit 64b4c33 Merge: efa88c3 1cdf0a4 Author: Tim Sullivan <tsullivan@users.noreply.github.com> Date: Wed May 4 11:08:44 2022 -0700 Merge branch 'main' into reporting/ks-2573 commit efa88c3 Author: Timothy Sullivan <tsullivan@elastic.co> Date: Wed May 4 11:07:47 2022 -0700 higher visualize png check threshold commit 8e76398 Merge: 366a28a 47f4658 Author: Timothy Sullivan <tsullivan@elastic.co> Date: Wed May 4 11:04:47 2022 -0700 Merge remote-tracking branch 'elastic/main' into reporting/ks-2573 commit 366a28a Author: Timothy Sullivan <tsullivan@elastic.co> Date: Wed May 4 10:52:32 2022 -0700 add visualize test for tsvb commit 7c7e00d Author: Timothy Sullivan <tsullivan@elastic.co> Date: Tue May 3 19:06:42 2022 -0700 add new test for kibana 7.6 sample data commit 25319dd Author: Timothy Sullivan <tsullivan@elastic.co> Date: Tue May 3 17:07:25 2022 -0700 add test fixtures for 7.6 dashboard

tsullivan · 2022-05-10T01:45:16Z

x-pack/test/functional/apps/dashboard/group1/reporting/screenshots.ts

@@ -227,5 +261,57 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
        await kibanaServer.uiSettings.replace({});
      });
    });
+
+    describe('Sample data from Kibana 7.6', () => {


The test adds sample data that was saved and imported in 7.6, as according to the original bug, the types of visualizations here tend to highlight the race condition more than Lens visualizations.

elasticmachine · 2022-05-10T01:47:57Z

Pinging @elastic/kibana-app-services (Team:AppServicesUx)

flash1293 · 2022-05-10T08:30:08Z

About the TSVB problem - when setting the viewport before calling the screenshot method the full panel would be captured for me: #131605 (comment) (it introduced a new issue of blurriness, not sure what that's about)

dokmic

Everything LGTM! That's a fantastic job 👍

kibana-ci · 2022-05-17T18:24:09Z

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`screenshotting`	3	5	+2

Total ESLint disabled count

id	before	after	diff
`screenshotting`	6	8	+2

History

💚 Build #45154 succeeded e1928ba
💚 Build #45113 succeeded 2eaff1d
💚 Build #44816 succeeded 85ec2a1
💚 Build #44745 succeeded 01b3034
💚 Build #44687 succeeded d22353e
💔 Build #44630 failed 0b1d84a

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

kibanamachine · 2022-05-20T00:26:20Z

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 131877 or prevent reminders by adding the backport:skip label.

tsullivan · 2022-05-20T19:07:14Z

Added the backport:skip label, since I manually staged the backport PRs

flash1293 · 2022-05-25T09:00:25Z

@tsullivan It seems like this PR missed the 7.17.4 build:

This is the last commit that went into 7.17.4: https://github.com/elastic/kibana/commits/a408358a8fc5671f5eb7985678a1733684441b37

This is the commit merging the reporting fix into the 7.17 branch: 4dd9bf1

I guess we should adjust the labels on this PR?
Should we make sure there is a 7.17.5 release in the near term to get this fix out?

bataya0 · 2022-05-31T09:35:23Z

I see that it also didnt make it into v8.2.2

tsullivan · 2022-05-31T21:17:12Z

@bataya0 I have downloaded the Docker build of 8.2.2 and tested it out. One of the symptoms of this bug was, you could not run a PDF/PNG report from TSVB editor, and I verified this test case. I also viewed the script code in the docker container to make sure it has the latest code, and it does.

Maybe you are having a different issue?

bataya0 · 2022-05-31T21:23:56Z

In v8.2.2 when viewing a TSVB item, I can still generate PDF and PNG successfully, so I guess the fix is in v8.2.2 even though the tag for v8.2.2 isnt on this pull request.

tsullivan · 2022-05-31T22:26:13Z

@bataya0 it looks like I needed to correct the version labels here. Done. Thanks for the heads up!

tsullivan force-pushed the screenshot/fix-resize-during-capture branch 2 times, most recently from 9d83b19 to c6969c3 Compare May 10, 2022 00:16

tsullivan mentioned this pull request May 10, 2022

[Reporting] When capturing screenshots only set viewport size for print-layout reports #125056

Closed

tsullivan force-pushed the screenshot/fix-resize-during-capture branch 2 times, most recently from 093d47a to 38167c0 Compare May 10, 2022 00:43

tsullivan mentioned this pull request May 10, 2022

[Reporting] Functional test with imported saved objects from 7.6 #131478

Closed

9 tasks

tsullivan added 4 commits May 9, 2022 18:22

[Screenshotting] Add captureBeyondViewport: false to workaround a res…

21dfdd0

…ize bug in Puppeteer

skip the visualize test

cbd1926

revert extra logging that broke tests

5ab1168

tsullivan force-pushed the screenshot/fix-resize-during-capture branch from 67d06a8 to 5ab1168 Compare May 10, 2022 01:22

remove baseline file for non-working test

3fdfd6f

elastic deleted a comment from kibana-ci May 10, 2022

tsullivan commented May 10, 2022

View reviewed changes

tsullivan marked this pull request as ready for review May 10, 2022 01:47

tsullivan requested review from a team as code owners May 10, 2022 01:47

revert a messy change

14fdb7c

tsullivan removed the request for review from a team May 10, 2022 01:55

This was referenced May 17, 2022

[screenshotting]fix resize during capture [8.2] #132295

Merged

[screenshotting]fix resize during capture [8.1] #132296

Closed

[screenshotting]fix resize during capture [8.0] #132297

Closed

cleanup unnecessary async

e1928ba

dokmic approved these changes May 17, 2022

View reviewed changes

Merge branch 'main' into screenshot/fix-resize-during-capture

3543568

tsullivan enabled auto-merge (squash) May 17, 2022 18:24

brianseeders disabled auto-merge May 17, 2022 18:36

brianseeders merged commit 8cf3344 into elastic:main May 17, 2022

tsullivan deleted the screenshot/fix-resize-during-capture branch May 17, 2022 19:07

tsullivan mentioned this pull request May 18, 2022

[Reporting] Job fails with params with non-numeric width or height #132380

Closed

tsullivan added v8.1.4 and removed v8.1.3 labels May 19, 2022

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label May 20, 2022

tsullivan added the backport:skip This commit does not require backporting label May 20, 2022

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label May 20, 2022

tsullivan removed the v8.1.4 label May 20, 2022

tsullivan added v8.2.2 v7.17.5 and removed v8.2.1 v7.17.4 labels May 31, 2022

tsullivan mentioned this pull request Jun 6, 2022

Kibana prints empty for TSVB gauge and metric charts, classical visualization types #131111

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Screenshotting] Add captureBeyondViewport: false to workaround a res… #131877

[Screenshotting] Add captureBeyondViewport: false to workaround a res… #131877

tsullivan commented May 9, 2022 •

edited

Loading

tsullivan May 10, 2022

elasticmachine commented May 10, 2022

flash1293 commented May 10, 2022 •

edited

Loading

dokmic left a comment

kibana-ci commented May 17, 2022

ESLint disabled line counts

Total ESLint disabled count

kibanamachine commented May 20, 2022

tsullivan commented May 20, 2022

flash1293 commented May 25, 2022

bataya0 commented May 31, 2022

tsullivan commented May 31, 2022

bataya0 commented May 31, 2022

tsullivan commented May 31, 2022

[Screenshotting] Add captureBeyondViewport: false to workaround a res… #131877

[Screenshotting] Add captureBeyondViewport: false to workaround a res… #131877

Conversation

tsullivan commented May 9, 2022 • edited Loading

Summary

captureBeyondViewport flag

Checklist

Release Note

tsullivan May 10, 2022

Choose a reason for hiding this comment

elasticmachine commented May 10, 2022

flash1293 commented May 10, 2022 • edited Loading

dokmic left a comment

Choose a reason for hiding this comment

kibana-ci commented May 17, 2022

💚 Build Succeeded

Metrics [docs]

ESLint disabled line counts

Total ESLint disabled count

History

kibanamachine commented May 20, 2022

tsullivan commented May 20, 2022

flash1293 commented May 25, 2022

bataya0 commented May 31, 2022

tsullivan commented May 31, 2022

bataya0 commented May 31, 2022

tsullivan commented May 31, 2022

tsullivan commented May 9, 2022 •

edited

Loading

flash1293 commented May 10, 2022 •

edited

Loading