Implement support for CommonJS 'require(moduleId)' API

detro commented 13 years ago

During testing that involve DOM Manipulation, it should be possible to simulate user clicks.

Something like: <code> phantom.click(CSS SELECTOR) </code>

Disclaimer: This issue was migrated on 2013-03-15 from the project's former issue tracker on Google Code, Issue #47. :star2: 22 people had starred this issue at the time of migration.

detro commented 13 years ago

detroniz...@gmail.com commented:

In the meantime, this works from the Javascript side: function simulateMouseClick(selector) { var targets = document.querySelectorAll(selector), evt = document.createEvent('MouseEvents'), i, len; evt.initMouseEvent("click", true, true, window, 0, 0, 0, 0, 0, false, false, false, false, 0, null);
for ( i = 0, len = targets.length; i < len; ++i ) { targets[i].dispatchEvent(evt); }
}

ariya commented 13 years ago

ariya.hi...@gmail.com commented:

Since this can be done via pure JavaScript, I don't see the pressing need for this.

What I do think as interesting is to create the events (not only mouse event) from the Qt side and propagate it via QWebFrame::event. That'll simulate the system events better, and more likely useful for testing.

detro commented 13 years ago

detroniz...@gmail.com commented:

Well, the Qt Events are powerful but a bit "too big" for what I think is the aim here.

The purpose is purely allowing the user to click programmatically on stuff.

But we could than evolve it into a more complete "event generation and usage" later. What do you recon?

detro commented 13 years ago

detroniz...@gmail.com commented:

Another reason for "pushing out" an "easy click" functionality, is that some people in the forum asked for such thing a couple of times. And indeed they end up using the above code.

Why not adding it for now?

We can work on something much more rich later on.

ariya commented 13 years ago

ariya.hi...@gmail.com commented:

I'm not opposed to the idea, it's just it's not urgent.

Again, since it can be implemented without the help from the native side, it should not be extremely important.

We can't really populate the official API with everything. If this kind of functionality can be done in pure JavaScript, then it should be available as a module.

detro commented 13 years ago

detroniz...@gmail.com commented:

OK. What do you have in mind than for this stuff? Can you formulate an example of possible api for events?

Ivan De Marino Front End Developer @ Betfair

Sent from my iPhone 4

shimopus commented 13 years ago

shimo...@gmail.com commented:

Ariya,

But how I can emulate click on buttons in alert and confirm dialogs?

And another questions. Do you know any modules that provide functionality from this issue?

ariya commented 13 years ago

ariya.hi...@gmail.com commented:

For events API, both DOM and Qt can be the source of inspiration.

Right now, nothing you can do on alert or confirm. They are not displayed as dialogs anyway.

detro commented 13 years ago

detroniz...@gmail.com commented:

Metadata Updates

Owner updated: detroniz...@gmail.com

detro commented 13 years ago

detroniz...@gmail.com commented:

Metadata Updates

Milestone updated: FutureRelease (was: ---)

ariya commented 13 years ago

eferonline@gmail.com commented:

Another great source of inspiration would be the Watir API: http://rdoc.info/gems/watir (as also used in clerity, firewatir, etc...) as it covers common scenarions in headles browser remote controlling.

Maybe also selenium could be of interest, since it is already supported by projects like http://code.google.com/p/webkitdriver/

ariya commented 13 years ago

ariya.hi...@gmail.com commented:

Related commit https://github.com/ariya/phantomjs/commit/049e1f50 (it needs API cleanup).

Metadata Updates

Milestone updated: Release1.3 (was: FutureRelease)

detro commented 13 years ago

detroniz...@gmail.com commented:

It looks good, but it might need a way to send the clicks using Query Selector, not coordinates. I recently had a request about this functionality but Betfair's QA Lead: it is way easier to automate clicks on elements using queries, instead of coordinates.

ariya commented 13 years ago

roejame...@gmail.com commented:

Perhaps an overload to accept coordinates AND elements. I know it can be done using Python, probably same on C++. We can keep both behaviors this way. :)

ariya commented 13 years ago

ariya.hi...@gmail.com commented:

But the selector-targeted event can be implement in pure JavaScript already.

Once we have CommonJS require, this can be in its own module, e.g. 'event'.

detro commented 13 years ago

detroniz...@gmail.com commented:

Metadata Updates

Milestone updated: FutureRelease (was: Release1.3)

detro commented 13 years ago

detroniz...@gmail.com commented:

Specs from: http://wiki.commonjs.org/wiki/Modules/1.1.1

Metadata Updates

Title updated: Implement support for CommonJS 'require(moduleId)' API

detro commented 13 years ago

detroniz...@gmail.com commented:

Metadata Updates

Milestone updated: Release1.3 (was: FutureRelease)

ariya commented 13 years ago

ariya.hi...@gmail.com commented:

There is also important semantic difference (I don't know if this is already solved in your pull request). If we would have enforced require('webpage'), the code should look like:

var WebPage = require('webpage'); var page = new WebPage(opts); ....

i.e. require can't just instantiate an object, it's just creating a module.

detro commented 13 years ago

detroniz...@gmail.com commented:

Mmm, not really sure about that.

See http://wiki.commonjs.org/wiki/Modules/1.1.1 and http://nodejs.org/docs/v0.4.11/api/fs.html as examples.

I think require should return an actually built object, not just a Constructor Function.

Also, i took some liberty (and this is where I really need some review), to accept a second parameter, to pass an object for the constructed.

ariya commented 13 years ago

ariya.hi...@gmail.com commented:

'fs' is static and therefore it's different.

Consider if we use factory pattern instead:

var WebPage = require('webpage');

var page1 = WebPage.create(options); page.doSomething();

var page2 = WebPage.create(options); page2.doThat();

detro commented 13 years ago

detroniz...@gmail.com commented:

That is doable too.

But it raises a concerne: WebPage for us will be a "reserved variable", as we will have the current "window.WebPage" set to a contructor function.

Maybe not a big deal, but to consider.

Anyway, taking your "factory" pattern, the code for "window.WebPage" will be something like:

window.WebPage = function(opts) { var webPageFactory = require('webpage'); return webPageFactory.create(opts); };

Right?

Ivan De Marino Front-End Developer @ Betfair

email: ivan.de.marino@gmail.com | detronizator@gmail.com | ivan.demarino@betfair.com web: blog.ivandemarino.me | www.linkedin.com/in/ivandemarino | twitter.com/detronizator mobile: +44 (0)7515 955 861

ariya commented 13 years ago

ariya.hi...@gmail.com commented:

What do you mean by "reserved variable"?

In all cases, the window.webPage that you give will break the existing scripts.

detro commented 13 years ago

detroniz...@gmail.com commented:

Nevermind the "reserved thing".

what do you mean with the other point?

Ivan De Marino Front End Developer @ Betfair

Sent from my iPhone 4

ariya commented 13 years ago

ariya.hi...@gmail.com commented:

I was thinking that WebPage is the factory (which has the 'create' method, as an alternative to new). That allows for a nice shortcut:

var page = require('webpage').create(opts);

detro commented 13 years ago

detroniz...@gmail.com commented:

To recap (I'm getting confused - must be the paracetamol):

To maintain backward compatibility, we will have 2 ways to create a page:

new WebPage(opts)

require('webpage').create(opts)

The first will be just a wrap around the second. Something like: window.WebPage = function(opts) { return require('webpage').create(opts); };

Agreed? Or am I missing something of what you are saying?

ariya commented 13 years ago

ariya.hi...@gmail.com commented:

Bingo!

ariya commented 13 years ago

roejame...@gmail.com commented:

Somewhat unrelated, but maybe we can add a way to use certain future settings, and have a future library. That way it can make the deprecation process easier, and people can still use the current version, but update their code, then maybe include a deprecated warning somewhere.

ariya commented 13 years ago

ariya.hi...@gmail.com commented:

For a generic module loading (not our built-in fs and webpage), I think a solution based on eval() will be too dangerous.

A better alternative I can think of is to wrap the module content in a function and then use call/apply. That way, we also have a chance to compile the content if it's CoffeeScript.

I'm not sure we can come up with a sensible solution (which also passes CommonJS tests) before 1.3.

detro commented 13 years ago

detroniz...@gmail.com commented:

I tried but I couldn't come up with a different approach (yet).

I actually HATE that "eval".

As an option, we could themporarily reduce "require" to accept only "fs" and "webpage" for now.

Ivan De Marino Front-End Developer @ Betfair

email: ivan.de.marino@gmail.com | detronizator@gmail.com | ivan.demarino@betfair.com web: blog.ivandemarino.me | www.linkedin.com/in/ivandemarino | twitter.com/detronizator mobile: +44 (0)7515 955 861

ariya commented 13 years ago

ariya.hi...@gmail.com commented:

Function actually accepts the function body: https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Function#Example:_Specifying_arguments_with_the_Function_constructor

But if we do that, we might as well use the native (C++) help by enclosing the module content in a function.

And yes, for 1.3 I guess we will fake 'fs' and 'webpage' only. I'll bump this to 1.4.

Metadata Updates

Milestone updated: Release1.4 (was: Release1.3)

detro commented 13 years ago

detroniz...@gmail.com commented:

I have been thinking about this, and I'm not really sure I still agree.

What's really wrong with using "eval()" in this context? Compared to the use of a Function constructor, there is no real difference: indeed I was wrapping the eval-ed content in a function call, with the "exports" variable provided in the scope. EXACTLY what I'd expect to do with in case we go for the Function constructor.

For the C++ "enclosing", I don't see how we can do that. The use of "evaluateJavascript" will cause us a jump of context that, ultimately, is more readable if we do the whole thing in Javascript instead.

ruiyang2012 commented 13 years ago

rui.y...@gmail.com commented:

Is it possible to add support for click on Flash Object, we tried to use the createEvent and dispatch it to the Flash DOM, but it won't work, the Flash Object does not get the event.

ariya commented 13 years ago

ariya.hi...@gmail.com commented:

I'm not sure which one is less evil: eval or function. The point I'm trying to make it that we should see how difficult it is to inject malicious code in the module. After all, that's the purpose of having the module (otherwise, we'll just use include, aka script tag solution).

detro commented 13 years ago

detroniz...@gmail.com commented:

There is the same possibility to inject "malicious" code as there is in including a malicious library. Require is not making any new thing: it's just a pattern of how to provide extra functionalities to your code.

Eval or Function are both ok to me. Maybe Function can be e bit "clearer", but ultimately they do exactly the same.

ariya commented 13 years ago

ariya.hi...@gmail.com commented:

Here is my 5-minute attempt. I'm sure we might be able to guard eval against the similar attack but it's pretty simple with Function. This does not mean that we can't attack Function at all.

var code = "exports.foo = 42; window.Math.random = function() { return 666; }";

// Using Function var f = new Function("exports", "window", code); var context = {}; var global = {}; var exports = { foo: 43 }; try { f.call(context, exports, global); console.log(exports.foo); console.log(Math.random()); } catch (e) { console.log('Exception ' + e.toString()); }

// Using eval var exports = { foo: 43 }; try { (function(exports) { eval(code); })(exports); console.log(exports.foo); console.log(Math.random()); } catch (e) { console.log('Exception ' + e.toString()); }

phantom.exit();

detro commented 13 years ago

detroniz...@gmail.com commented:

Indeed you can pass "window" and "this" to the eval, set to empty objects, to ensure the same kind of screening.

But I do agree with "Function" it makes it clearer.

Ivan De Marino Front-End Developer @ Betfair

email: ivan.de.marino@gmail.com | detronizator@gmail.com | ivan.demarino@betfair.com web: blog.ivandemarino.me | www.linkedin.com/in/ivandemarino | twitter.com/detronizator mobile: +44 (0)7515 955 861

ariya commented 13 years ago

ariya.hi...@gmail.com commented:

I haven't seen a proven way to limit the 'this' scope with eval, though. But then, I'm still new to this crazy dynamic language :)

ariya commented 12 years ago

ariya.hi...@gmail.com commented:

Not enough time to resolve for 1.4. Postpone to 1.5.

Metadata Updates

Milestone updated: Release1.5 (was: Release1.4)

JamieMason commented 12 years ago

siunmaru@gmail.com commented:

+1 for any way at all to support splitting code run on $ phantomjs into their own files/modules. CommonJS' require() would be ideal, but anything would be welcome at the moment as I'm working with 1 pretty large file.

Please let me know if I've missed anything. I've seen libraryPath but that appears to be related to injecting code into remote pages.

Thanks again Ariya

@GotNoSugarBaby

execjosh commented 12 years ago

I haven't thought too hard about this, but would it not be possible to do something along the lines of the JSON config loader?

jgonera commented 12 years ago

jgon...@gmail.com commented:

Is there any progress? Will this get implemented for 1.5? In case someone is interested, you can try using something I created for this purpose for now: https://github.com/jgonera/phantomjs-nodify Among other things it implements CommonJS require(). I don't know if it is fully specs compliant and it may have some bugs, but it works for me (tested on PhantomJS 1.3 and 1.4.1).

ariya commented 12 years ago

ariya.hi...@gmail.com commented:

This has to be postponed to next version.

Metadata Updates

Milestone updated: FutureRelease (was: Release1.5)

detro commented 12 years ago

detroniz...@gmail.com commented:

Issue 120 has been merged into this issue.

detro commented 12 years ago

detroniz...@gmail.com commented:

Issue 152 has been merged into this issue.

Metadata Updates

CC(s) added:
- ariya.hi...@gmail.com

detro commented 12 years ago

detroniz...@gmail.com commented:

Extract from #152, to keep all the references in one issue:

... In the specific case, the specs I'd like to see implemented in PhantomJS are:

CommonJS/Modules --- http://wiki.commonjs.org/wiki/Modules

CommonJS/Packages --- http://wiki.commonjs.org/wiki/Packages

Development notes:

We already have all the "backend" functionalities to load external libraries

It might turn out to be very simple to implement this using almost pure Javascript ...

jgonera commented 12 years ago

jgon...@gmail.com commented:

It might turn out to be very simple to implement this using almost pure Javascript

Indeed, that's what I did in phantomjs-nodify (comment #42).

detro commented 12 years ago

detroniz...@gmail.com commented:

Well, than you should:

make sure it complies with the CommonJS Moduel specs

submit a pull request

enjoy the glory :P

jgonera commented 12 years ago

jgon...@gmail.com commented:

I refactored my code and wrote tests to make it more reliable before thinking about merging it into PhantomJS. Unfortunately, I discovered a bug in PhantomJS 1.5 which as of now makes my modules much less useful because when an exception is thrown I'm unable to track down which module threw it (http://code.google.com/p/phantomjs/issues/detail?id=510).

Also, I would like to know if we absolutely need cycles support at this point. My solution does not support cycles as of now (and I guess I won't have time to implement it in the nearest future).

I'd be glad if someone could take a look at my code (relevant stuff is in the patchRequire() function): https://github.com/jgonera/phantomjs-nodify/blob/master/nodify.js#L45

detro commented 12 years ago

detroniz...@gmail.com commented:

Hey, from a quick view your code it seems fine.

Why don't you submit the pull request and assume that eventually the issue with the exception reporting will be fixed?

I'd really love to adopt your "require" in GhostDriver: otherwise, I'll have to find another solution :(

QUESTION: Does your require allow a module to use require inside? I mean, can module B depend on module A, and both use "require()"?

ariya / phantomjs

Implement support for CommonJS 'require(moduleId)' API #10047

Extract from #152, to keep all the references in one issue:

It might turn out to be very simple to implement this using almost pure Javascript ...