Skip to content

Commit

Permalink
fix(screenshots): throw on 0x0 screenshots (#3756)
Browse files Browse the repository at this point in the history
Fix #2672
  • Loading branch information
JoelEinbinder authored and aslushnikov committed Jan 11, 2019
1 parent 29a2438 commit 96adedf
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 3 deletions.
2 changes: 2 additions & 0 deletions lib/ExecutionContext.js
Expand Up @@ -504,6 +504,8 @@ class ElementHandle extends JSHandle {

boundingBox = await this.boundingBox();
assert(boundingBox, 'Node is either not visible or not an HTMLElement');
assert(boundingBox.width !== 0, 'Node has 0 width.');
assert(boundingBox.height !== 0, 'Node has 0 height.');

const { layoutViewport: { pageX, pageY } } = await this._client.send('Page.getLayoutMetrics');

Expand Down
2 changes: 2 additions & 0 deletions lib/Page.js
Expand Up @@ -841,6 +841,8 @@ class Page extends EventEmitter {
assert(typeof options.clip.y === 'number', 'Expected options.clip.y to be a number but found ' + (typeof options.clip.y));
assert(typeof options.clip.width === 'number', 'Expected options.clip.width to be a number but found ' + (typeof options.clip.width));
assert(typeof options.clip.height === 'number', 'Expected options.clip.height to be a number but found ' + (typeof options.clip.height));
assert(options.clip.width !== 0, 'Expected options.clip.width not to be 0.');
assert(options.clip.height !== 0, 'Expected options.clip.width not to be 0.');
}
return this._screenshotTaskQueue.postTask(this._screenshotTask.bind(this, screenshotType, options));
}
Expand Down
7 changes: 4 additions & 3 deletions test/screenshot.spec.js
Expand Up @@ -218,10 +218,11 @@ module.exports.addTests = function({testRunner, expect, product}) {
const screenshotError = await elementHandle.screenshot().catch(error => error);
expect(screenshotError.message).toBe('Node is either not visible or not an HTMLElement');
});
xit('should not hang with zero width/height element', async({page, server}) => {
await page.setContent('<div style="width: 0; height: 0"></div>');
it('should not hang with zero width/height element', async({page, server}) => {
await page.setContent('<div style="width: 50px; height: 0"></div>');
const div = await page.$('div');
await div.screenshot();
const error = await div.screenshot().catch(e => e);
expect(error.message).toBe('Node has 0 height.');
});
});

Expand Down

0 comments on commit 96adedf

Please sign in to comment.