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

feat(chromium): roll Chromium to r843427 #6797

Merged
merged 13 commits into from
Feb 2, 2021
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
"license": "Apache-2.0",
"dependencies": {
"debug": "^4.1.0",
"devtools-protocol": "0.0.818844",
"devtools-protocol": "0.0.847576",
"extract-zip": "^2.0.0",
"https-proxy-agent": "^5.0.0",
"node-fetch": "^2.6.1",
Expand Down
2 changes: 1 addition & 1 deletion src/revisions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,6 @@ type Revisions = Readonly<{
}>;

export const PUPPETEER_REVISIONS: Revisions = {
chromium: '818858',
chromium: '848005',
firefox: 'latest',
};
6 changes: 3 additions & 3 deletions test/ariaqueryhandler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -563,13 +563,13 @@ describeChromeOnly('AriaQueryHandler', () => {
const { page } = getTestState();
const found = await page.$$('aria/[role="heading"]');
const ids = await getIds(found);
expect(ids).toEqual(['shown', 'hidden', 'node11', 'node13']);
expect(ids).toEqual(['shown', 'node11', 'node13']);
});
it('should find both ignored and unignored', async () => {
it('should not find ignored', async () => {
const { page } = getTestState();
const found = await page.$$('aria/title');
const ids = await getIds(found);
expect(ids).toEqual(['shown', 'hidden']);
expect(ids).toEqual(['shown']);
});
});
});
11 changes: 11 additions & 0 deletions test/cookies.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ describe('Cookie specs', () => {
value: 'John Doe',
domain: 'localhost',
path: '/',
sameParty: false,
expires: -1,
size: 16,
httpOnly: false,
Expand Down Expand Up @@ -99,6 +100,7 @@ describe('Cookie specs', () => {
value: '1234',
domain: 'localhost',
path: '/',
sameParty: false,
expires: -1,
size: 12,
httpOnly: false,
Expand All @@ -110,6 +112,7 @@ describe('Cookie specs', () => {
value: 'John Doe',
domain: 'localhost',
path: '/',
sameParty: false,
expires: -1,
size: 16,
httpOnly: false,
Expand Down Expand Up @@ -145,6 +148,7 @@ describe('Cookie specs', () => {
value: 'tweets',
domain: 'baz.com',
path: '/',
sameParty: false,
expires: -1,
size: 11,
httpOnly: false,
Expand All @@ -156,6 +160,7 @@ describe('Cookie specs', () => {
value: 'woofs',
domain: 'foo.com',
path: '/',
sameParty: false,
expires: -1,
size: 10,
httpOnly: false,
Expand Down Expand Up @@ -248,6 +253,7 @@ describe('Cookie specs', () => {
value: '123456',
domain: 'localhost',
path: '/',
sameParty: false,
expires: -1,
size: 14,
httpOnly: false,
Expand All @@ -271,6 +277,7 @@ describe('Cookie specs', () => {
value: 'GRID',
domain: 'localhost',
path: '/grid.html',
sameParty: false,
expires: -1,
size: 14,
httpOnly: false,
Expand Down Expand Up @@ -376,6 +383,7 @@ describe('Cookie specs', () => {
value: 'best',
domain: 'www.example.com',
path: '/',
sameParty: false,
expires: -1,
size: 18,
httpOnly: false,
Expand Down Expand Up @@ -414,6 +422,7 @@ describe('Cookie specs', () => {
value: 'best',
domain: 'localhost',
path: '/',
sameParty: false,
expires: -1,
size: 20,
httpOnly: false,
Expand All @@ -428,6 +437,7 @@ describe('Cookie specs', () => {
value: 'worst',
domain: '127.0.0.1',
path: '/',
sameParty: false,
expires: -1,
size: 15,
httpOnly: false,
Expand Down Expand Up @@ -479,6 +489,7 @@ describe('Cookie specs', () => {
value: 'best',
domain: '127.0.0.1',
path: '/',
sameParty: false,
expires: -1,
size: 24,
httpOnly: false,
Expand Down
3 changes: 3 additions & 0 deletions test/defaultbrowsercontext.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ describe('DefaultBrowserContext', function () {
value: 'John Doe',
domain: 'localhost',
path: '/',
sameParty: false,
expires: -1,
size: 16,
httpOnly: false,
Expand All @@ -62,6 +63,7 @@ describe('DefaultBrowserContext', function () {
value: 'John Doe',
domain: 'localhost',
path: '/',
sameParty: false,
expires: -1,
size: 16,
httpOnly: false,
Expand Down Expand Up @@ -93,6 +95,7 @@ describe('DefaultBrowserContext', function () {
value: '1',
domain: 'localhost',
path: '/',
sameParty: false,
expires: -1,
size: 8,
httpOnly: false,
Expand Down
Binary file modified test/golden-chromium/screenshot-offscreen-clip.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
14 changes: 13 additions & 1 deletion test/page.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ describe('Page', function () {
expect(await page.evaluate(() => !!window.opener)).toBe(false);
expect(await popup.evaluate(() => !!window.opener)).toBe(false);
});
it('should work with clicking target=_blank', async () => {
it('should work with clicking target=_blank and without rel=opener', async () => {
const { page, server } = getTestState();

await page.goto(server.EMPTY_PAGE);
Expand All @@ -186,6 +186,18 @@ describe('Page', function () {
page.click('a'),
]);
expect(await page.evaluate(() => !!window.opener)).toBe(false);
expect(await popup.evaluate(() => !!window.opener)).toBe(false);
});
it('should work with clicking target=_blank and with rel=opener', async () => {
const { page, server } = getTestState();

await page.goto(server.EMPTY_PAGE);
await page.setContent('<a target=_blank rel=opener href="/one-style.html">yo</a>');
const [popup] = await Promise.all([
new Promise<Page>((x) => page.once('popup', x)),
page.click('a'),
]);
expect(await page.evaluate(() => !!window.opener)).toBe(false);
expect(await popup.evaluate(() => !!window.opener)).toBe(true);
});
it('should work with fake-clicking target=_blank and rel=noopener', async () => {
Expand Down
8 changes: 4 additions & 4 deletions test/screenshot.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,15 @@ describe('Screenshots', function () {
});
expect(screenshot).toBeGolden('screenshot-clip-rect.png');
});
itFailsFirefox('should clip elements to the viewport', async () => {
itFailsFirefox('should clip elements to the viewport size', async () => {
const { page, server } = getTestState();

await page.setViewport({ width: 500, height: 500 });
await page.setViewport({ width: 50, height: 50 });
await page.goto(server.PREFIX + '/grid.html');
const screenshot = await page.screenshot({
clip: {
x: 50,
y: 600,
x: 0,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why would not the old test work?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to what I got from the CL 2643792, it used to cut the screenshot respecting the ViewPort position, but now the viewport moves to the clip first, and then taking screenshot. So the initial ViewPort offset doesn't matter anymore.
Actual output of the initial version of the test: https://i.imgur.com/QC2rTB5.png

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is a breaking change in how Puppeteer's clip works?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it is.
Another breaking change is in the AriaQuery

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mentioned in the initial commit.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, it's the breaking change in how the screenshot works. The clip is still respected in both offset and size. But the result screenshot is filled with white without respect of the initial ViewPort offset, only based on its size.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, there are some problems with the respecting offset in the screenshot.
EG this example produces the incorrectly-looking result: https://i.imgur.com/syIImrg.png

await page.setViewport({ width: 50, height: 50 });
await page.goto(server.PREFIX + '/grid.html');
const screenshot = await page.screenshot({
  clip: {
    x: 25,
    y: 25,
    width: 100,
    height: 100,
  },
});

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, thanks. Btw, per https://www.conventionalcommits.org/en/v1.0.0/#summary we should use this format:

BREAKING CHANGE: …

y: 0,
width: 100,
height: 100,
},
Expand Down
1 change: 1 addition & 0 deletions versions.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
const versionsPerRelease = new Map([
// This is a mapping from Chromium version => Puppeteer version.
// In Chromium roll patches, use 'NEXT' for the Puppeteer version.
['90.0.4403.0', 'NEXT'],
['88.0.4298.0', 'v5.5.0'],
['87.0.4272.0', 'v5.4.0'],
['86.0.4240.0', 'v5.3.0'],
Expand Down