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

docs(api): document appMode #1954

Closed
wants to merge 1 commit into from
Closed

docs(api): document appMode #1954

wants to merge 1 commit into from

Conversation

ebidel
Copy link
Contributor

@ebidel ebidel commented Feb 2, 2018

I've found appMode useful when using pptr with lighthouse.

Background:
Both Lighthouse and Puppeteer emulate a viewport when you run the tools, but they use different sizes. This causes lighthouse's perf audits to fail because screenshots end up changing sizes during an audit. During an audit session, Lighthouse reloads the page several times.

example - inject a stylesheet into the page before each lighthouse runs.

const {URL} = require('url');
const puppeteer = require('puppeteer');
const lighthouse = require('lighthouse');

(async() => {

const url = 'https://www.chromestatus.com/features';

const browser = await puppeteer.launch({
  appMode: true, // sets headless: true and does not set a viewport size. Let ligthhouse do that.
});

browser.on('targetchanged', async target => {
  const page = await target.page();
  if (page && page.url() === url) {
  await page.addStyleTag({content: 'body {color: red}'});
  }
});

const lhr = await lighthouse(url, {
  port: (new URL(browser.wsEndpoint())).port,
  output: 'json',
  logLevel: 'info',
  // disableDeviceEmulation: true,
});

console.log(`Lighthouse score: ${lhr.score}`);

await browser.close();

})();

@alixaxel
Copy link
Contributor

alixaxel commented Feb 5, 2018

I had to find out about this the hard way.

It would be useful to be able to not set a default viewport size and still be able to have headless setting. Related #1910.

@alixaxel alixaxel mentioned this pull request Feb 5, 2018
@aslushnikov
Copy link
Contributor

@alixaxel @ebidel would the page.setViewport(null) to disable viewport emulation help you?

@ebidel
Copy link
Contributor Author

ebidel commented Feb 5, 2018

I think one of the issues we see is that that users have to remember to override the viewport in the first place. It's a source of confusion and easy to forget. It would also be nice to not have to make an API call for every page.

@aslushnikov
Copy link
Contributor

I think one of the issues we see is that that users have to remember to override the viewport in the first place. It's a source of confusion and easy to forget. It would also be nice to not have to make an API call for every page.

@ebidel I think this would always be the case: If we don't override the viewport by default, it's size will be unpredictable.

@alixaxel
Copy link
Contributor

alixaxel commented Feb 5, 2018

@aslushnikov For me, the appMode is confusing because it does too many things:

https://github.com/GoogleChrome/puppeteer/blob/e998ac93250cc7279043831328b6f4f49bc244bb/lib/Launcher.js#L68-L71

https://github.com/GoogleChrome/puppeteer/blob/e998ac93250cc7279043831328b6f4f49bc244bb/lib/Launcher.js#L83-L90

And then Page will set the viewport:

https://github.com/GoogleChrome/puppeteer/blob/e998ac93250cc7279043831328b6f4f49bc244bb/lib/Page.js#L57-L58

In summary, setting appMode: true has the effect of:

  • turning off headless mode
  • ignoring the option of ignoreDefaultArgs
  • not setting the viewport size to the default 800x600
  • not using AUTOMATION_ARGS:

https://github.com/GoogleChrome/puppeteer/blob/e998ac93250cc7279043831328b6f4f49bc244bb/lib/Launcher.js#L50-L54

In my case, I just want the viewport (in headful) to always be set to the resolution that I expect.

@ebidel In your example you have:

appMode: true, // sets headless: true and does not set a viewport size. Let ligthhouse do that.

Which doesn't agree with the docs you wrote (correct version):

- `appMode` <[boolean]> If this option is set to `true`, the browser opens in headful mode and Puppeteer does not set a default viewport of 800x600 for new pages. Overrides the `headless` option if both options are used together.

@aslushnikov
Copy link
Contributor

@aslushnikov For me, the appMode is confusing because it does too many things

@alixaxel that's true. appMode is purely experimental and is a subject for removal; that's the reason it's not documented yet (and shouldn't be).

I'm not sure I understand if the page.setViewport(null) would help you. Is it enough?

@alixaxel
Copy link
Contributor

alixaxel commented Feb 5, 2018

@aslushnikov I don't know what's the behavior/idea of page.setViewport(null) TBH. Basically, it would be helpful to find a solution for the inconsistency in #1910, which (I assume) will also affect Lighthouse.

If page.setViewport(null) would do the trick, great!

@ebidel
Copy link
Contributor Author

ebidel commented Feb 5, 2018

@ebidel I think this would always be the case: If we don't override the viewport by default, it's size will be unpredictable.

Guess I'm unclear why we set a viewport. Chrome seems to be fine without setting one viewport:

const puppeteer = require('puppeteer');
(async () => {
  const browser = await puppeteer.launch({appMode: true});
  const page = await browser.newPage();
  await page.goto('https://www.facebook.com/');
  await page.screenshot({path: 'screenshot.png'});
  await browser.close();
})();

That produces a screenshot where the page/viewport take up the full window dimensions. That feels like the right behavior over pptr setting one for users. It's also more like the rest of the API (less magic, you opt-in to things).

Does chrome not launch with the same dimensions across platforms?

@JoelEinbinder
Copy link
Collaborator

It doesn't launch with the same dimensions across headful/headless. I can't imagine its the same across platform.

@ebidel
Copy link
Contributor Author

ebidel commented Feb 13, 2018

Can we make a decision on this?

TLDR: we need some way to prevent pptr from setting a default viewport. It would be nice if that was a flag and not an explicit API call. The default 800x600 is a source of gotcha/bugs for us.

BTW, does page.setViewport(null) currently work?

@aslushnikov
Copy link
Contributor

Can we make a decision on this?

appMode is a highly experimental feature that serves as our playground; it's subject to change every other day and shouldn't be relied upon.

The default 800x600 is a source of gotcha/bugs for us.

I'm not sure I follow this. Would it be better if there are random numbers instead?

BTW, does page.setViewport(null) currently work?

It doesn't.

Closing this in favor of #1910

@ebidel
Copy link
Contributor Author

ebidel commented Feb 13, 2018

Would it be better if there are random numbers instead?

No, it would be better if pptr didn't set a default viewport at all for users. Seems to work just fine. People run into issues with responsiveness: e.g. site not matching selectors, page layout being different for mobile, etc. It's usually b/c they don't realize we're setting a small viewport at launch.

@paulirish
Copy link
Collaborator

No, it would be better if pptr didn't set a default viewport at all for users. Seems to work just fine. People run into issues with responsiveness: e.g. site not matching selectors, page layout being different for mobile, etc. It's usually b/c they don't realize we're setting a small viewport at launch.

We discussed this more offline. Many sites are clipped when screenshotted by puppeteer in a user's first hello world script.

There's a proposal for changing the default viewport width from it's current 800x600. A fresh profile headful Chrome launches at 1200px wide, so we think that's a good new width. (height TBD, not as big a deal) This will require a major version bump, so it should go onto a list of things to change at the 2.0. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants