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

Add .content() method. Fixes #406. #419

Merged
merged 1 commit into from
Aug 21, 2017

Conversation

jeresig
Copy link
Contributor

@jeresig jeresig commented Aug 20, 2017

This adds a new method: .content() which is the counterpart of .setContent(), retrieving the HTML contents of the page, including the doctype (if there is one).

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
  • In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

@jeresig
Copy link
Contributor Author

jeresig commented Aug 20, 2017

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@vsemozhetbyt
Copy link
Contributor

Does XMLSerializer API have any restrictions to not use it instead?

@vsemozhetbyt
Copy link
Contributor

See also: #406 (comment)

@RobertCorey
Copy link

RobertCorey commented Aug 20, 2017

Thanks so much! This has been my first time interacting with an open source library and it's been really great being supported.

@jeresig
Copy link
Contributor Author

jeresig commented Aug 20, 2017

@vsemozhetbyt Oh, I'm not aware of any! I could give that a try for just the doctype, simplifying the logic a bit. I wanted to avoid using it for the full document as it then became an XML document (which can be pretty different from what was originally intended). I'll update the PR to serialize the doctype.

docs/api.md Outdated
@@ -462,6 +463,11 @@ Shortcut for [page.mainFrame().focus(selector)](#framefocusselector).
#### page.frames()
- returns: <[Array]<[Frame]>> An array of all frames attached to the page.

#### page.getContent()
Copy link
Contributor

Choose a reason for hiding this comment

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

let's go with page.content(), we don't have "get" prefixes in other API calls (e.g. page.plainText()).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aslushnikov thank you for the review! I've just updated it to be .content() instead of .getContent().

@jeresig jeresig changed the title Add .getContent() method. Fixes #406. Add .content() method. Fixes #406. Aug 21, 2017
Copy link
Contributor

@aslushnikov aslushnikov left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

@aslushnikov aslushnikov merged commit 598f439 into puppeteer:master Aug 21, 2017
@jeresig
Copy link
Contributor Author

jeresig commented Aug 21, 2017

You're very welcome! Thank you for the review!

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.

5 participants