add features for asynchronous downloads

[jefleponot]

To solve asynschronous downloads that does work with phantomjs v2 (issue : Synchronous XHR causing zero byte file downloads #1342 ), there is a proposal PR

for @langholz and @istr , thanks to your specifications.

PS : operation synchronously javascript is simply an absurdity because it inhibits the language spirit.
PS : PS : Could it be possible that a great tools as CasperJS be written to use asynchronous methods
(http://phantomjs.org/api/webpage/method/evaluate-async.html)
We could use it in more domains like "chrome extension tabs" : https://developer.chrome.com/extensions/tabs#method-executeScript

Best Regards
jef

[istr]

I would never ever change the API behavior (i.e. default values) depending on the engine in use.

Mark this "obsolete / not recommended" in doc, throw a warning to the output when used and advise to use the newly created "waitForDownload" instead.

Better throw an error if used with an engine that does not support it than silently changing behavior here.

[istr]

@jefleponot Thanks a lot for this PR 👍 .

It seems that you have a regression with form selects in your patch. Some builds fail for receipt of JSON content.

Please:

• have a look into what causes the regression(s) and fix it
• lint the code (some builds fail due to linting)

I would never silently change the API behavior due to the engine version in use, though.
Could you re-work your PR along my proposal and

• just keep behavior for the sync functions
• mark them obsolete / not recommended in the doc and give a hint to use the new waitFor... functions instead?
• maybe throw an error within the non-recommended functions when used with an engine that does not support them?

I would really like to integrate this PR into 1.1 because it is a major improvement.

[jefleponot]

your are totaly right. You would never ever change the API behavior (i.e. default values) depending on the engine in use. The code will now throw an error.

I find my regression trouble with meld diff-tools.
I will do my best to make a PR soon.

Thanks for all your brillant analyses.

[jefleponot]

partial PR done. CU next time
could you help me @istr ? Only one test can't success.

Thanks in advance

Sorry it works now :-)
That is really curious

[istr]

Nice! Thanks a lot. Restarted the failing job (known intermittent popup bug).

Will review, ask a second collaborator for opinion and then merge.

[jefleponot]

I have done a lot of improvements of CasperJS for my web service supervision job, that I newer commit. if you are interested in lots of PR, it will valid them.

[istr]

@jefleponot Sounds good. Maybe it would be best to create separate feature branches to help make the different change-sets small.

Any improvement is welcome. Especially those that help to close the numerous open issues.

Thanks for your support.

[mickaelandrieu]

PhantomJS instead please :)

[mickaelandrieu]

@jefleponot great contribution ! some comments to address then. But I'm 👍 to merge this feature :)

[istr]

@mickaelandrieu thanks for quick review. :) Maybe let's discuss the question:

• silently change API behavior based on engine in use

vs.

• warn the user she's trying to do something that won't be supported in the future

approach with @n1k0, @hexid, and @paazmaya...

EDIT (see also discussion at #1342)

[jefleponot]

Another issue for you @istr : casperJS has never used "window.callPhantom" method before this PR.
It could be better to normalize this remote operation.

"window.callPhantom" is similar to the "window.postMessage" function : postMessage(data, targetDomain)

So it could be great to use the same object attributes

The special propertys of a message event are:

data
    The first argument of postMessage
origin
    The source domain
source
    The reference to sending window if it is possible to 
respond by calling event.source.postMessage

chrome extensions has yet normalize communications with the "message passing" :
https://developer.chrome.com/extensions/messaging
Message Passing is really useful to communicate between web page and "background page" (one to one) And also between many pages. It avoid to fall victim to cross-site scripting.

In the modules/casper.js file a senMessage method could be add like the chrome.tabs.sendMessage : sendMessage(integer tabId, any message, object options, function responseCallback)

Thanks for yours advices

[mickaelandrieu]

@istr sounds OK to me 👍

[eloitay]

@jefleponot I tried patching this change into the source code myself and notice that it still produce zero byte files. So I am wondering if the way the script is written need to be changed as well in order for the bug to be fixed.

[jefleponot]

Hi @eloitay
Could you send me you code ? I could help you more

Or you can do a PR with an example.

[paazmaya]

This would give the impression that the third parameter is required when the second is given.
Perhaps (String url [, String method [, Object data, Boolean async ] ] )

[jefleponot]

thanks

[BIGjuevos]

Do we want to keep this issue in the 1.1 release @istr @mickaelandrieu? I vote to move to a new milestone 1.2.

I feel like this would be good for a 1.2 considering our focus on squashing bugs for 1.1. This is something I'd be happy to pick up in the future. Feel free to assign to me.

[mickaelandrieu]

Humm ok for 1.2, how many issues are blocking the release of 1.1 according to the actual milestone?

[jefleponot]

test ok - PhantomJS 1.9.7 -> 2.1.1
test ok - SlimmerJS 0.8.4 -> 0.9.6

result : 6748 octets

code :

var casper = require("casper").create();

casper.start("http://www.google.fr/", function() {
    this.download("http://www.google.fr/images/srpr/logo3w.png", "logo.png",'GET',{},true);
});
casper.then(function(){
    this.waitForDownload("http://www.google.fr/images/srpr/logo3w.png",function(){
        console.log('download sucess');
    }, function(){
        console.log('download failed');
    });
});
casper.run();

It is fully functional , works in production since January 2016 with phantomJS 2.1.1 because old download method can't be used

[jefleponot]

@mickaelandrieu : update done

[mickaelandrieu]

@jefleponot sadly, your last merged pull request have been merged and so on you need to rebase this one 💃

@casperjs/contributors so, are we agree to merge this one? my vote is 👍

[istr]

I would prefer not to break the API in this way:

• do not introduce boolean parameters
• especially do not introduce boolean parameters that change the previous "default" behaviour
• do not make the default behaviour / value dependent on the engine version in use

See https://github.com/casperjs/casperjs/issues/1342#issuecomment-207848750 what I would like to see in this PR instead:

• mark the current (broken) API functions obsolete and add a "broken" comment to the docs
• add a warning to those functions that say: "this is broken and will not work; it is obsolete and will be removed"
• create a new set of functions that implements what this PR achieves

[jefleponot]

@istr,

Thanks for your explanation.

you are right, this PR don't suit for many reasons.
JavaScript programming is now moving towards a purely asynchronous programming.

Synchronous XMLHttpRequest on the main thread is deprecated because of its detrimental effects to the end user's experience. For more help, check https://xhr.spec.whatwg.org/.

With PhantomJS and SlimerJS, on the opposite, methods are mixed synchronous or asynchronous :

• open(url, callback) - asynchronous
• evaluateAsync(function, [delayMillis, arg1, arg2, ...]) - asynchronous
• ==> page.evaluate(function, arg1, arg2, ...) - synchronous
• ==> page.render (filename [, {format, quality}]) - synchronous
• ==> page.renderBase64(format) - synchronous

If you look for an equivalent approach like chrome.tabs, every methods are fully asynchronous :

• executeScript(integer tabId, object details, function callback)
• captureVisibleTab(integer windowId, object options, function callback)
  (https://developer.chrome.com/extensions/tabs)

CasperJS has same trouble as PhantomJS and SlimerJS : mix synchronous and asynchronous fct.
wait(), waitFor(), waitForAlert(), waitForPopup() ... - asynchronous
withFrame(), withPopup() - asynchronous

download() - synchronous
evaluate() - synchronous

The use of Synchronous mode, although it eases writing limits treatments.

@istr, could we propose a new set of asynchronous functions what could lead CasperJS to good javascript practices ?

[mickaelandrieu]

@jefleponot @istr I agree with Jef, and we need to keep the old api.

Could be better for now to create new functions instead of improve the existing one (and this may be a good start for 2.0 api :) )

[sairoko12]

Someone what happened with this request?

[kensoh]

Archiving PR using ARCHIVE label as CasperJS, like PhantomJS, is not actively maintained for now. These PRs are from 1-5 years ago but there just hasn't been maintainers to continue the work.