[Reporting] set viewport to include clip area (#87253)

* [Reporting] set viewport to include clip area * remove getViewport * fix tests * simpler * fix 1 * revert * hacks * scope the logging variables * polish * hacky fix * quieter logging * make less hacky * fix functional test * revert lowering log level of browser console messages * revise comments * setViewport only to happen once * fix snapshot of layout type tests * fix comment text * Revert "setViewport only to happen once" This reverts commit 15977f9db4. * fix disgusting bug * use x/y ordering for width/height * fix fn test snapshots Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
2021-01-06 15:30:35 -07:00 · 2021-01-06 15:30:35 -07:00 · 5d135359d7
parent 693775ce03
commit 5d135359d7
14 changed files with 91 additions and 18 deletions
--- a/x-pack/plugins/reporting/server/browsers/chromium/driver/chromium_driver.ts
+++ b/x-pack/plugins/reporting/server/browsers/chromium/driver/chromium_driver.ts
@ -196,14 +196,17 @@ export class HeadlessChromiumDriver {
  }

  public async setViewport(
-    { width, height, zoom }: ViewZoomWidthHeight,
+    { width: _width, height: _height, zoom }: ViewZoomWidthHeight,
    logger: LevelLogger
  ): Promise<void> {
-    logger.debug(`Setting viewport to width: ${width}, height: ${height}, zoom: ${zoom}`);
+    const width = Math.floor(_width);
+    const height = Math.floor(_height);
+
+    logger.debug(`Setting viewport to: width=${width} height=${height} zoom=${zoom}`);

    await this.page.setViewport({
-      width: Math.floor(width / zoom),
-      height: Math.floor(height / zoom),
+      width,
+      height,
      deviceScaleFactor: zoom,
      isMobile: false,
    });
@ -243,7 +246,7 @@ export class HeadlessChromiumDriver {
      }

      if (this._shouldUseCustomHeaders(conditionalHeaders.conditions, interceptedUrl)) {
-        logger.debug(`Using custom headers for ${interceptedUrl}`);
+        logger.trace(`Using custom headers for ${interceptedUrl}`);
        const headers = map(
          {
            ...interceptedRequest.request.headers,
@ -270,7 +273,7 @@ export class HeadlessChromiumDriver {
        }
      } else {
        const loggedUrl = isData ? this.truncateUrl(interceptedUrl) : interceptedUrl;
-        logger.debug(`No custom headers for ${loggedUrl}`);
+        logger.trace(`No custom headers for ${loggedUrl}`);
        try {
          await client.send('Fetch.continueRequest', { requestId });
        } catch (err) {
--- a/x-pack/plugins/reporting/server/browsers/chromium/driver_factory/args.ts
+++ b/x-pack/plugins/reporting/server/browsers/chromium/driver_factory/args.ts
@ -41,6 +41,9 @@ export const args = ({ userDataDir, viewport, disableSandbox, proxy: proxyConfig
    '--disable-gpu',
    '--headless',
    '--hide-scrollbars',
+    // NOTE: setting the window size does NOT set the viewport size: viewport and window size are different.
+    // The viewport may later need to be resized depending on the position of the clip area.
+    // These numbers come from the job parameters, so this is a close guess.
    `--window-size=${Math.floor(viewport.width)},${Math.floor(viewport.height)}`,
  ];

--- a/x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.ts
+++ b/x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.ts
@ -65,6 +65,7 @@ export class HeadlessChromiumDriverFactory {
      logger.info(`Creating browser page driver`);

      const chromiumArgs = this.getChromiumArgs(viewport);
+      logger.debug(`Chromium launch args set to: ${chromiumArgs}`);

      let browser: puppeteer.Browser;
      let page: puppeteer.Page;
--- a/x-pack/plugins/reporting/server/export_types/printable_pdf/lib/generate_pdf.ts
+++ b/x-pack/plugins/reporting/server/export_types/printable_pdf/lib/generate_pdf.ts
@ -43,6 +43,7 @@ export async function generatePdfObservableFactory(reporting: ReportingCore) {
    tracker.startLayout();

    const layout = createLayout(captureConfig, layoutParams);
+    logger.debug(`Layout: width=${layout.width} height=${layout.height}`);
    tracker.endLayout();

    tracker.startScreenshots();
--- a/x-pack/plugins/reporting/server/lib/layouts/canvas_layout.ts
+++ b/x-pack/plugins/reporting/server/lib/layouts/canvas_layout.ts
@ -64,8 +64,8 @@ export class CanvasLayout extends Layout implements LayoutInstance {

  public getViewport() {
    return {
-      height: this.scaledHeight,
-      width: this.scaledWidth,
+      height: this.height,
+      width: this.width,
      zoom: ZOOM,
    };
  }
--- a/x-pack/plugins/reporting/server/lib/layouts/layout.ts
+++ b/x-pack/plugins/reporting/server/lib/layouts/layout.ts
@ -33,6 +33,8 @@ export abstract class Layout {
    pageSizeParams: PageSizeParams
  ): CustomPageSize | PredefinedPageSize;

+  // Return the dimensions unscaled dimensions (before multiplying the zoom factor)
+  // driver.setViewport() Adds a top and left margin to the viewport, and then multiplies by the scaling factor
  public abstract getViewport(itemsCount: number): ViewZoomWidthHeight | null;

  public abstract getBrowserZoom(): number;
--- a/x-pack/plugins/reporting/server/lib/layouts/preserve_layout.ts
+++ b/x-pack/plugins/reporting/server/lib/layouts/preserve_layout.ts
@ -51,8 +51,8 @@ export class PreserveLayout extends Layout implements LayoutInstance {

  public getViewport() {
    return {
-      height: this.scaledHeight,
-      width: this.scaledWidth,
+      height: this.height,
+      width: this.width,
      zoom: ZOOM,
    };
  }
--- a/x-pack/plugins/reporting/server/lib/level_logger.ts
+++ b/x-pack/plugins/reporting/server/lib/level_logger.ts
@ -49,6 +49,10 @@ export class LevelLogger implements GenericLevelLogger {
    this.getLogger(tags).debug(msg);
  }

+  public trace(msg: string, tags: string[] = []) {
+    this.getLogger(tags).trace(msg);
+  }
+
  public info(msg: string, tags: string[] = []) {
    this.getLogger(tags).info(trimStr(msg));
  }
--- a/x-pack/plugins/reporting/server/lib/screenshots/constants.ts
+++ b/x-pack/plugins/reporting/server/lib/screenshots/constants.ts
@ -7,6 +7,7 @@
 export const DEFAULT_PAGELOAD_SELECTOR = '.application';

 export const CONTEXT_GETNUMBEROFITEMS = 'GetNumberOfItems';
+export const CONTEXT_GETBROWSERDIMENSIONS = 'GetBrowserDimensions';
 export const CONTEXT_INJECTCSS = 'InjectCss';
 export const CONTEXT_WAITFORRENDER = 'WaitForRender';
 export const CONTEXT_GETTIMERANGE = 'GetTimeRange';
--- a/x-pack/plugins/reporting/server/lib/screenshots/get_screenshots.ts
+++ b/x-pack/plugins/reporting/server/lib/screenshots/get_screenshots.ts
@ -7,10 +7,60 @@
 import { i18n } from '@kbn/i18n';
 import { LevelLogger, startTrace } from '../';
 import { HeadlessChromiumDriver } from '../../browsers';
+import { LayoutInstance } from '../layouts';
 import { ElementsPositionAndAttribute, Screenshot } from './';
+import { CONTEXT_GETBROWSERDIMENSIONS } from './constants';
+
+// In Puppeteer 5.4+, the viewport size limits what the screenshot can take, even if a clip is specified. The clip area must
+// be visible in the viewport. This workaround resizes the viewport to the actual content height and width.
+// NOTE: this will fire a window resize event
+const resizeToClipArea = async (
+  item: ElementsPositionAndAttribute,
+  browser: HeadlessChromiumDriver,
+  zoom: number,
+  logger: LevelLogger
+) => {
+  // Check current viewport size
+  const { width, height, left, top } = item.position.boundingClientRect; // the "unscaled" pixel sizes
+  const [viewWidth, viewHeight] = await browser.evaluate(
+    {
+      fn: () => [document.body.clientWidth, document.body.clientHeight],
+      args: [],
+    },
+    { context: CONTEXT_GETBROWSERDIMENSIONS },
+    logger
+  );
+
+  logger.debug(`Browser viewport: width=${viewWidth} height=${viewHeight}`);
+
+  // Resize the viewport if the clip area is not visible
+  if (viewWidth < width + left || viewHeight < height + top) {
+    logger.debug(`Item's position is not within the viewport.`);
+
+    // add left and top margin to unscaled measurements
+    const newWidth = width + left;
+    const newHeight = height + top;
+
+    logger.debug(
+      `Resizing browser viewport to: width=${newWidth} height=${newHeight} zoom=${zoom}`
+    );
+
+    await browser.setViewport(
+      {
+        width: newWidth,
+        height: newHeight,
+        zoom,
+      },
+      logger
+    );
+  }
+
+  logger.debug(`Capturing item: width=${width} height=${height} left=${left} top=${top}`);
+};

 export const getScreenshots = async (
  browser: HeadlessChromiumDriver,
+  layout: LayoutInstance,
  elementsPositionAndAttributes: ElementsPositionAndAttribute[],
  logger: LevelLogger
 ): Promise<Screenshot[]> => {
@ -25,6 +75,8 @@ export const getScreenshots = async (
  for (let i = 0; i < elementsPositionAndAttributes.length; i++) {
    const endTrace = startTrace('get_screenshots', 'read');
    const item = elementsPositionAndAttributes[i];
+
+    await resizeToClipArea(item, browser, layout.getBrowserZoom(), logger);
    const base64EncodedData = await browser.screenshot(item.position);

    screenshots.push({
--- a/x-pack/plugins/reporting/server/lib/screenshots/observable.test.ts
+++ b/x-pack/plugins/reporting/server/lib/screenshots/observable.test.ts
@ -243,10 +243,10 @@ describe('Screenshot Observable Pipeline', () => {
                      "attributes": Object {},
                      "position": Object {
                        "boundingClientRect": Object {
-                          "height": 200,
+                          "height": 100,
                          "left": 0,
                          "top": 0,
-                          "width": 200,
+                          "width": 100,
                        },
                        "scroll": Object {
                          "x": 0,
@ -271,10 +271,10 @@ describe('Screenshot Observable Pipeline', () => {
                      "attributes": Object {},
                      "position": Object {
                        "boundingClientRect": Object {
-                          "height": 200,
+                          "height": 100,
                          "left": 0,
                          "top": 0,
-                          "width": 200,
+                          "width": 100,
                        },
                        "scroll": Object {
                          "x": 0,
@ -339,6 +339,8 @@ describe('Screenshot Observable Pipeline', () => {

        if (mockCall === contexts.CONTEXT_ELEMENTATTRIBUTES) {
          return Promise.resolve(null);
+        } else if (mockCall === contexts.CONTEXT_GETBROWSERDIMENSIONS) {
+          return Promise.resolve([800, 600]);
        } else {
          return Promise.resolve();
        }
--- a/x-pack/plugins/reporting/server/lib/screenshots/observable.ts
+++ b/x-pack/plugins/reporting/server/lib/screenshots/observable.ts
@ -87,6 +87,7 @@ export function screenshotsObservableFactory(
              }),
              mergeMap(() => getNumberOfItems(captureConfig, driver, layout, logger)),
              mergeMap(async (itemsCount) => {
+                // set the viewport to the dimentions from the job, to allow elements to flow into the expected layout
                const viewport = layout.getViewport(itemsCount) || getDefaultViewPort();
                await Promise.all([
                  driver.setViewport(viewport, logger),
@ -133,7 +134,7 @@ export function screenshotsObservableFactory(
                  const elements = data.elementsPositionAndAttributes
                    ? data.elementsPositionAndAttributes
                    : getDefaultElementPosition(layout.getViewport(1));
-                  const screenshots = await getScreenshots(driver, elements, logger);
+                  const screenshots = await getScreenshots(driver, layout, elements, logger);
                  const { timeRange, error: setupError } = data;
                  return {
                    timeRange,
--- a/x-pack/plugins/reporting/server/test_helpers/create_mock_browserdriverfactory.ts
+++ b/x-pack/plugins/reporting/server/test_helpers/create_mock_browserdriverfactory.ts
@ -64,6 +64,9 @@ mockBrowserEvaluate.mockImplementation(() => {
  if (mockCall === contexts.CONTEXT_GETNUMBEROFITEMS) {
    return Promise.resolve(1);
  }
+  if (mockCall === contexts.CONTEXT_GETBROWSERDIMENSIONS) {
+    return Promise.resolve([600, 800]);
+  }
  if (mockCall === contexts.CONTEXT_INJECTCSS) {
    return Promise.resolve();
  }
--- a/x-pack/test/functional/apps/canvas/reports.ts
+++ b/x-pack/test/functional/apps/canvas/reports.ts
@ -183,7 +183,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
          "
        `);

-        expectSnapshot(res.get('content-length')).toMatchInline(`"20726"`);
+        expect(res.get('content-length')).to.be('20725');
      });

      it('downloaded PDF base64 string is correct without borders and logo', async function () {
@ -327,12 +327,12 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
        /Filter /FlateDecode
        /ColorSpace /DeviceRGB
        /SMask 14 0 R
-        /Length 18
+        /Length 17
        >>
        "
        `);

-        expectSnapshot(res.get('content-length')).toMatchInline(`"1599"`);
+        expect(res.get('content-length')).to.be('1598');
      });
    });
  });