Added an option for adding key/values to phjs' page.customHeaders

[pconerly]

Yo,

I added an option to add custom headers to phantomjs.

Additionally I added:

• The tests to prove it works (including a small expressjs server to bounce headers).
• A listener in the Gruntfile.js to print debug info. (Not required, just useful.)
• Some documentation about options for the README.md

[cowboy]

Can you reformat headers_server.js to follow the established convention for variable declarations?

Also, I'm a slightly concerned about the precedent for specifying custom options here. Should this option that directly maps to the underlying customHeaders property be called customHeaders ?

What if we structure the options differently, such that there's a page option with a customHeaders sub-property? Then every property in the page object could (maybe) be enumerated and "passed through" to page.

Thoughts?

[pconerly]

Re: established convention for var declarations: Definitely. (I prefer coffeescript so I just ran it through js2coffee)

I agree with the change to make it an option format like:

options: {
    timeout: 1234,
    inject: 'fdsf',
    page: {
        customHeaders: {
            'x-custom': '1234'
         }
     }

I'll do both those changes. I'll also make the page option attempt to pass in all properties set in it.

Did you have any thoughts about the tiny expressjs server sitting in the test/ directory? I suspect that future tests may also need to use a tiny express server and I wasn't sure if there was a better way to re-organize it.

Thanks for the quick response!

[pconerly]

Pushed those changes.

[sindresorhus]

_.extend()

[pconerly]

fixed

[pconerly]

Thanks @sindresorhus, I made those changes.

[jakerella]

Hmm, adding the code to add arbitrary properties on the page object may also take care of my PR which explicitly added the viewportSize option to the page object (see PR #31).

@pconerly, could you verify that the page.viewportSize is set correctly using your code changes? If so, I'll cancel my PR. Thanks!!

[pconerly]

Hi @jakerella, Yup, it does pass on any page properties listed here: https://github.com/ariya/phantomjs/wiki/API-Reference-WebPage

I added a test for viewport size so you can be assured. You can close #31. You're welcome!

[jakerella]

Bump... May I ask why this one is waiting to be merged? We use this functionality for viewportSize on the page in every project (useful for responsive testing).

[dklischies]

Just in case somebody is looking for a more unobtrusive way to do this (e.g. no extra libraries required): https://github.com/Nostraa/grunt-lib-phantomjs/commit/54b4cc693e22e47836484a7d193d3001c7c851a2
It doesn't have any testing, but I guess that isn't required at all since stuff is only passed to phantomjs, not sure what should go wrong in 4 lines of code.

[ssafejava]

Could this please be merged? I have tests that rely on document.getElementAtPoint() that are failing because the viewport size is too small.

[pconerly]

@cowboy @sindresorhus , can we merge?

[ssafejava]

By the way, rather than include lodash, this can be simply fixed by adding options.page to the page constructor.

I pulled this fork & removed the lodash code and the tests still pass. Let me know if I can help get this fix into master.

[bdowling]

Curious if there is any concerns holding this PR from merging. I noticed some tests jQuery-ui was doing that could benefit from the ability to extend viewportSize as well. Their dialog tests are disabled in grunt/phantomjs I'm guessing in part because these won't pass..

[bdowling]

Fwiw, I made a couple minor mods to this pull request in my branch here. bdowling@1593476

[jzaefferer]

Closing in favor of #21, which improves on this.