fix(screenshot): wait for stable position before taking element screenshot (#3216)

Same goes for scrollIntoViewIfNeeded.

docs/actionability.md

@@ -11,7 +11,8 @@ Some actions like `page.click()` support `{force: true}` option that disable non
 | `check()`<br>`click()`<br>`dblclick()`<br>`hover()`<br>`uncheck()` | [Visible]<br>[Stable]<br>[Enabled]<br>[Receiving Events]<br>[Attached] |
 | `fill()` | [Visible]<br>[Enabled]<br>[Editable]<br>[Attached] |
 | `dispatchEvent()`<br>`focus()`<br>`press()`<br>`setInputFiles()`<br>`selectOption()`<br>`type()` | [Attached] |
-| `selectText()`<br>`scrollIntoViewIfNeeded()`<br>`screenshot()` | [Visible]<br>[Attached] |
+| `scrollIntoViewIfNeeded()`<br>`screenshot()` | [Visible]<br>[Stable]<br>[Attached] |
+| `selectText()` | [Visible]<br>[Attached] |
 | `getAttribute()`<br>`innerText()`<br>`innerHTML()`<br>`textContent()` | [Attached] |
 
 ### Visible

src/dom.ts

@@ -209,7 +209,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
 
   async _waitAndScrollIntoViewIfNeeded(progress: Progress): Promise<void> {
     while (progress.isRunning()) {
-      assertDone(throwRetargetableDOMError(await this._waitForVisible(progress)));
+      assertDone(throwRetargetableDOMError(await this._waitForDisplayedAtStablePosition(progress, false /* waitForEnabled */)));
 
       progress.throwIfAborted();  // Avoid action that has side-effects.
       const result = throwRetargetableDOMError(await this._scrollRectIntoViewIfNeeded());
@@ -321,7 +321,7 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
     if ((options as any).__testHookBeforeStable)
       await (options as any).__testHookBeforeStable();
     if (!force) {
-      const result = await this._waitForDisplayedAtStablePositionAndEnabled(progress);
+      const result = await this._waitForDisplayedAtStablePosition(progress, true /* waitForEnabled */);
       if (result !== 'done')
         return result;
     }
@@ -636,15 +636,21 @@ export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> {
     return result;
   }
 
-  async _waitForDisplayedAtStablePositionAndEnabled(progress: Progress): Promise<'error:notconnected' | 'done'> {
-    progress.logger.info('  waiting for element to be visible, enabled and not moving');
+  async _waitForDisplayedAtStablePosition(progress: Progress, waitForEnabled: boolean): Promise<'error:notconnected' | 'done'> {
+    if (waitForEnabled)
+      progress.logger.info(`  waiting for element to be visible, enabled and not moving`);
+    else
+      progress.logger.info(`  waiting for element to be visible and not moving`);
     const rafCount =  this._page._delegate.rafCountForStablePosition();
-    const poll = this._evaluateHandleInUtility(([injected, node, rafCount]) => {
-      return injected.waitForDisplayedAtStablePositionAndEnabled(node, rafCount);
-    }, rafCount);
+    const poll = this._evaluateHandleInUtility(([injected, node, { rafCount, waitForEnabled }]) => {
+      return injected.waitForDisplayedAtStablePosition(node, rafCount, waitForEnabled);
+    }, { rafCount, waitForEnabled });
     const pollHandler = new InjectedScriptPollHandler(progress, await poll);
     const result = await pollHandler.finish();
-    progress.logger.info('  element is visible, enabled and does not move');
+    if (waitForEnabled)
+      progress.logger.info('  element is visible, enabled and does not move');
+    else
+      progress.logger.info('  element is visible and does not move');
     return result;
   }
 

src/injected/injectedScript.ts

@@ -422,7 +422,7 @@ export default class InjectedScript {
     input.dispatchEvent(new Event('change', { 'bubbles': true }));
   }
 
-  waitForDisplayedAtStablePositionAndEnabled(node: Node, rafCount: number): types.InjectedScriptPoll<'error:notconnected' | 'done'> {
+  waitForDisplayedAtStablePosition(node: Node, rafCount: number, waitForEnabled: boolean): types.InjectedScriptPoll<'error:notconnected' | 'done'> {
     let lastRect: types.Rect | undefined;
     let counter = 0;
     let samePositionCounter = 0;
@@ -464,7 +464,7 @@ export default class InjectedScript {
       const isVisible = !!style && style.visibility !== 'hidden';
 
       const elementOrButton = element.closest('button, [role=button]') || element;
-      const isDisabled = ['BUTTON', 'INPUT', 'SELECT'].includes(elementOrButton.nodeName) && elementOrButton.hasAttribute('disabled');
+      const isDisabled = waitForEnabled && ['BUTTON', 'INPUT', 'SELECT'].includes(elementOrButton.nodeName) && elementOrButton.hasAttribute('disabled');
 
       if (isDisplayed && isStable && isVisible && !isDisabled)
         return 'done';

test/assets/grid.html

@@ -49,4 +49,14 @@ body {
 ::-webkit-scrollbar {
     display: none;
 }
+
+@keyframes move {
+    from { left: -500px; background-color: cyan; }
+    to { left: 0; background-color: rgb(255, 210, 204); }
+}
+.box.animation {
+    position: relative;
+    animation: 2s linear 0s move forwards;
+}
+
 </style>

test/elementhandle.jest.js

@@ -361,6 +361,21 @@ describe('ElementHandle.scrollIntoViewIfNeeded', function() {
     await page.setContent('<span style="display:none"><div>Hello</div></span>');
     await testWaiting(page, div => div.parentElement.style.display = 'block');
   });
+  it('should wait for element to stop moving', async({page, server}) => {
+    await page.setContent(`
+    <style>
+      @keyframes move {
+        from { margin-left: 0; }
+        to { margin-left: 200px; }
+      }
+      div.animated {
+        animation: 2s linear 0s infinite alternate move;
+      }
+    </style>
+    <div class=animated>moving</div>
+    `);
+    await testWaiting(page, div => div.classList.remove('animated'));
+  });
 
   it('should timeout waiting for visible', async({page, server}) => {
     await page.setContent('<div style="display:none">Hello</div>');

test/screenshot.jest.js

@@ -537,4 +537,19 @@ describe.skip(ffheadful)('ElementHandle.screenshot', function() {
     await utils.verifyViewport(page, 350, 360);
     await context.close();
   });
+  it('should wait for element to stop moving', async({page, server}) => {
+    await page.setViewportSize({ width: 500, height: 500 });
+    await page.goto(server.PREFIX + '/grid.html');
+    const elementHandle = await page.$('.box:nth-of-type(3)');
+    await elementHandle.evaluate(e => e.classList.add('animation'));
+    const screenshot = await elementHandle.screenshot();
+    expect(screenshot).toBeGolden('screenshot-element-bounding-box.png');
+  });
+  it('should take screenshot of disabled button', async({page}) => {
+    await page.setViewportSize({ width: 500, height: 500 });
+    await page.setContent(`<button disabled>Click me</button>`);
+    const button = await page.$('button');
+    const screenshot = await button.screenshot();
+    expect(screenshot).toBeInstanceOf(Buffer);
+  });
 });