Skip to content
This repository was archived by the owner on May 30, 2023. It is now read-only.

Changing webpage settings should get immediate result #13660

Merged
3 commits merged into from
Jan 8, 2018

Conversation

iradul
Copy link
Contributor

@iradul iradul commented Oct 16, 2015

This will fix #13659
It will make sure that when page.settings property is changed it will take effect immediately.

@zackw
Copy link
Contributor

zackw commented Oct 16, 2015

Thanks for tackling this problem!

Unfortunately, we can't take your patch as is, because it changes a documented API and would break existing controller scripts. The third argument to page.openUrl needs to stay, even though it's no longer needed internally. Can you make that adjustment? It looks like it should be pretty simple.

Also, please consider writing some test cases.

@iradul
Copy link
Contributor Author

iradul commented Oct 16, 2015

Original page.openUrl is reverted back and I also added a simple test case.

@vitallium
Copy link
Collaborator

I got infinite recursion with this PR on creating a web page :(

@iradul
Copy link
Contributor Author

iradul commented Oct 20, 2015

Can you elaborate ?
For example this works perfectly fine after clean build, tested only on Windows though:

var page = require('webpage').create();
console.log( page.evaluate(function() { return navigator.userAgent; }) );
page.settings.userAgent = "Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/46.0.2490.71 Safari/537.36";
console.log( page.evaluate(function() { return navigator.userAgent; }) ); 
page.open("http://google.com", function(status) {
    console.log(status);
    phantom.exit();
})
Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/538.1 (KHTML, like Gecko) PhantomJS/2.0.0 Safari/538.1
Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/46.0.2490.71 Safari/537.36
success

@vitallium vitallium force-pushed the master branch 2 times, most recently from 1842ba8 to 842715b Compare March 15, 2016 17:40
@vitallium vitallium force-pushed the master branch 2 times, most recently from e024f31 to 5d99f2a Compare May 19, 2016 20:53
@rmehner
Copy link

rmehner commented Oct 16, 2016

We're running into the same issue (we need to be able to disable JavaScript for security reasons, and we use setContent). Is there anything I can do to help out with this to get this merged and into a PhantomJS release? :)

@ghost ghost merged commit b846133 into ariya:master Jan 8, 2018
ghost pushed a commit that referenced this pull request Jan 8, 2018
ghost pushed a commit that referenced this pull request Jan 8, 2018
ghost pushed a commit that referenced this pull request Jan 8, 2018
@rumblefrog rumblefrog mentioned this pull request Mar 4, 2018
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Changing webpage settings takes effect only after page.open is called
4 participants