Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(page): fix page.#scrollIntoViewIfNeeded method #8631

Merged
merged 1 commit into from Jul 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
68 changes: 35 additions & 33 deletions src/common/ElementHandle.ts
Expand Up @@ -240,50 +240,52 @@ export class ElementHandle<

async #scrollIntoViewIfNeeded(this: ElementHandle<Element>): Promise<void> {
const error = await this.evaluate(
async (element, pageJavascriptEnabled): Promise<string | false> => {
async (element): Promise<string | undefined> => {
if (!element.isConnected) {
return 'Node is detached from document';
}
if (element.nodeType !== Node.ELEMENT_NODE) {
return 'Node is not of type HTMLElement';
}
// force-scroll if page's javascript is disabled.
if (!pageJavascriptEnabled) {
element.scrollIntoView({
block: 'center',
inline: 'center',
// @ts-expect-error Chrome still supports behavior: instant but
// it's not in the spec so TS shouts We don't want to make this
// breaking change in Puppeteer yet so we'll ignore the line.
behavior: 'instant',
});
return false;
}
const visibleRatio = await new Promise(resolve => {
const observer = new IntersectionObserver(entries => {
resolve(entries[0]!.intersectionRatio);
observer.disconnect();
});
observer.observe(element);
});
if (visibleRatio !== 1.0) {
element.scrollIntoView({
block: 'center',
inline: 'center',
// @ts-expect-error Chrome still supports behavior: instant but
// it's not in the spec so TS shouts We don't want to make this
// breaking change in Puppeteer yet so we'll ignore the line.
behavior: 'instant',
});
}
return false;
},
this.#page.isJavaScriptEnabled()
return;
OrKoN marked this conversation as resolved.
Show resolved Hide resolved
}
);

if (error) {
throw new Error(error);
}

try {
await this._client.send('DOM.scrollIntoViewIfNeeded', {
OrKoN marked this conversation as resolved.
Show resolved Hide resolved
objectId: this._remoteObject.objectId,
});
} catch (_err) {
// Fallback to Element.scrollIntoView if DOM.scrollIntoViewIfNeeded is not supported
await this.evaluate(
async (element, pageJavascriptEnabled): Promise<void> => {
const visibleRatio = async () => {
return await new Promise(resolve => {
const observer = new IntersectionObserver(entries => {
resolve(entries[0]!.intersectionRatio);
observer.disconnect();
});
observer.observe(element);
});
};
if (!pageJavascriptEnabled || (await visibleRatio()) !== 1.0) {
element.scrollIntoView({
block: 'center',
inline: 'center',
// @ts-expect-error Chrome still supports behavior: instant but
// it's not in the spec so TS shouts We don't want to make this
// breaking change in Puppeteer yet so we'll ignore the line.
behavior: 'instant',
});
}
},
this.#page.isJavaScriptEnabled()
);
}
}

async #getOOPIFOffsets(
Expand Down
5 changes: 4 additions & 1 deletion test/src/click.spec.ts
Expand Up @@ -164,7 +164,10 @@ describe('Page.click', function () {
await page.goto(server.PREFIX + '/offscreenbuttons.html');
const messages: any[] = [];
page.on('console', msg => {
return messages.push(msg.text());
if (msg.type() === 'log') {
OrKoN marked this conversation as resolved.
Show resolved Hide resolved
return messages.push(msg.text());
}
return;
OrKoN marked this conversation as resolved.
Show resolved Hide resolved
});
for (let i = 0; i < 11; ++i) {
// We might've scrolled to click a button - reset to (0, 0).
Expand Down
2 changes: 1 addition & 1 deletion test/src/elementhandle.spec.ts
Expand Up @@ -203,7 +203,7 @@ describe('ElementHandle specs', function () {
})
).toBe(true);
});
it('should work for TextNodes', async () => {
it('should not work for TextNodes', async () => {
const {page, server} = getTestState();

await page.goto(server.PREFIX + '/input/button.html');
Expand Down