Skip to content

Commit a45e22c

Browse files
committed
[Reporting] set viewport to include clip area
1 parent 9c092d5 commit a45e22c

File tree

4 files changed

+45
-16
lines changed

4 files changed

+45
-16
lines changed

x-pack/plugins/reporting/server/browsers/chromium/driver/chromium_driver.ts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ interface WaitForSelectorOpts {
2727

2828
interface EvaluateOpts {
2929
fn: EvaluateFn;
30-
args: SerializableOrJSHandle[];
30+
args?: SerializableOrJSHandle[];
3131
}
3232

3333
interface EvaluateMetaOpts {
@@ -195,18 +195,14 @@ export class HeadlessChromiumDriver {
195195
}
196196
}
197197

198+
// NOTE: dimensions must be given in the base pixels sizes, not pre-scaled by the zoom factor.
198199
public async setViewport(
199200
{ width, height, zoom }: ViewZoomWidthHeight,
200201
logger: LevelLogger
201202
): Promise<void> {
202203
logger.debug(`Setting viewport to width: ${width}, height: ${height}, zoom: ${zoom}`);
203204

204-
await this.page.setViewport({
205-
width: Math.floor(width / zoom),
206-
height: Math.floor(height / zoom),
207-
deviceScaleFactor: zoom,
208-
isMobile: false,
209-
});
205+
await this.page.setViewport({ width, height, deviceScaleFactor: zoom, isMobile: false });
210206
}
211207

212208
private registerListeners(conditionalHeaders: ConditionalHeaders, logger: LevelLogger) {

x-pack/plugins/reporting/server/browsers/chromium/driver_factory/index.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,11 @@ export class HeadlessChromiumDriverFactory {
6363
return Rx.Observable.create(async (observer: InnerSubscriber<any, any>) => {
6464
const logger = pLogger.clone(['browser-driver']);
6565
logger.info(`Creating browser page driver`);
66+
logger.debug(`viewport: ${JSON.stringify(viewport)}`);
67+
logger.debug(`viewport: ${browserTimezone}`);
6668

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

6972
let browser: puppeteer.Browser;
7073
let page: puppeteer.Page;

x-pack/plugins/reporting/server/lib/screenshots/get_screenshots.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,12 @@
77
import { i18n } from '@kbn/i18n';
88
import { LevelLogger, startTrace } from '../';
99
import { HeadlessChromiumDriver } from '../../browsers';
10+
import { LayoutInstance } from '../layouts';
1011
import { ElementsPositionAndAttribute, Screenshot } from './';
1112

1213
export const getScreenshots = async (
1314
browser: HeadlessChromiumDriver,
15+
layout: LayoutInstance,
1416
elementsPositionAndAttributes: ElementsPositionAndAttribute[],
1517
logger: LevelLogger
1618
): Promise<Screenshot[]> => {
@@ -25,6 +27,37 @@ export const getScreenshots = async (
2527
for (let i = 0; i < elementsPositionAndAttributes.length; i++) {
2628
const endTrace = startTrace('get_screenshots', 'read');
2729
const item = elementsPositionAndAttributes[i];
30+
31+
// In Puppeteer 5.4+, the viewport size limits what the screenshot can take, even if a clip is specified. The clip area must
32+
// be visible in the viewport. This workaround resizes the viewport to the actual content height and width.
33+
// NOTE: this will fire a window resize event
34+
35+
// Check current viewport size
36+
const [height, width] = await browser.evaluate(
37+
{
38+
fn: () => [document.body.clientHeight, document.body.clientWidth],
39+
},
40+
{ context: 'resize for screenshot' },
41+
logger
42+
);
43+
44+
logger.debug(`Browser viewport: height: ${height}, width: ${width}`);
45+
46+
if (
47+
height < item.position.boundingClientRect.height + item.position.boundingClientRect.top ||
48+
width < item.position.boundingClientRect.width + item.position.boundingClientRect.left
49+
) {
50+
// Resize viewport
51+
await browser.setViewport(
52+
{
53+
height: item.position.boundingClientRect.height + item.position.boundingClientRect.top,
54+
width: item.position.boundingClientRect.width + item.position.boundingClientRect.left,
55+
zoom: layout.getBrowserZoom(),
56+
},
57+
logger
58+
);
59+
}
60+
2861
const base64EncodedData = await browser.screenshot(item.position);
2962

3063
screenshots.push({

x-pack/plugins/reporting/server/lib/screenshots/observable.ts

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -86,13 +86,9 @@ export function screenshotsObservableFactory(
8686
);
8787
}),
8888
mergeMap(() => getNumberOfItems(captureConfig, driver, layout, logger)),
89-
mergeMap(async (itemsCount) => {
90-
const viewport = layout.getViewport(itemsCount) || getDefaultViewPort();
91-
await Promise.all([
92-
driver.setViewport(viewport, logger),
93-
waitForVisualizations(captureConfig, driver, itemsCount, layout, logger),
94-
]);
95-
}),
89+
mergeMap(async (itemsCount) =>
90+
waitForVisualizations(captureConfig, driver, itemsCount, layout, logger)
91+
),
9692
mergeMap(async () => {
9793
// Waiting till _after_ elements have rendered before injecting our CSS
9894
// allows for them to be displayed properly in many cases
@@ -132,8 +128,9 @@ export function screenshotsObservableFactory(
132128

133129
const elements = data.elementsPositionAndAttributes
134130
? data.elementsPositionAndAttributes
135-
: getDefaultElementPosition(layout.getViewport(1));
136-
const screenshots = await getScreenshots(driver, elements, logger);
131+
: getDefaultElementPosition(getDefaultViewPort());
132+
133+
const screenshots = await getScreenshots(driver, layout, elements, logger);
137134
const { timeRange, error: setupError } = data;
138135
return {
139136
timeRange,

0 commit comments

Comments
 (0)