sheebz / phantom-proxy

a lightweight proxy that lets you to drive phantomjs from node.
MIT License
137 stars 33 forks source link

No apparent way to access page settings #25

Closed AndrewRayCode closed 11 years ago

AndrewRayCode commented 11 years ago

for example, if I want to set the userAgent

I tried adding a regular plain old settings object to the page

        //sets property value ex. viewportSize
        _.extend(this, {
            settings: {},

but those settings do not translate to the phantomjs page.

I tried something like this:

                request.post(self.options.hostAndPort + '/page/settings/set', {
                        form:{
                            propertyName:propertyName,
                            propertyValue:propertyValue
                        }
                    },

but that just hangs forever. can you tell me where things like this are documented:

 '/page/properties/get'

Is that a phantom exposed thing? I can't find any documentation about it.

sheebz commented 11 years ago

Well, there's page.set - which allows you to specify the property name, and value. It's documented in the page object, also in unit tests:

example:

            it('should set clipRect', function (done) {
                phantomProxy.create({"debug":true}, function (proxy) {
                    should.exist(proxy.page);
                    proxy.page.set('clipRect', { top:14, left:3, width:400, height:300 }, function (result) {
                        assert.equal(result, true);
                        done();
                    });
                });
            });
AndrewRayCode commented 11 years ago

This ticket is fixed by the PR you merged, https://github.com/sheebz/phantom-proxy/pull/27

sheebz commented 11 years ago

Yeah, I did merge, can you verify that page.set('settings', {}) does not work? If it does work, I think we should revert this pull request.

AndrewRayCode commented 11 years ago

ah. that does appear to work. I suppose it could be slightly annoying if you want to set only one page setting, you'd have to get the whole thing, then modify the object, then re-set it, otherwise you'd clobber previously set settings. for that reason I argue for keeping it. but I leave it up to you. If you revert, I will make a separate PR that includes the 2 new tests, and update the readme to explicitly state how to change the settings, because I did not realize it was possible, and without really digging into the source code, I don't think a regular user would figure it out either.

AndrewRayCode commented 11 years ago

after sleeping on it, I personally think we should leave both options in, and document the way to set multiple settings at once, and a single setting (using this pr). this change does not remove or change any existing functionality, so there's no reason to remove it.

sheebz commented 11 years ago

Ok, I think thats fine