Skip to content
This repository has been archived by the owner on Sep 7, 2022. It is now read-only.

fix: Switched from launching phantomjs via node to ensuring that the .exe is always launched #33

Merged
merged 1 commit into from Apr 4, 2014
Merged

fix: Switched from launching phantomjs via node to ensuring that the .exe is always launched #33

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 11 additions & 17 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,25 @@
var fs = require('fs');
var path = require('path');


function serializeOption(value) {
if (typeof value === 'function') {
return value.toString();
}
return JSON.stringify(value);
}

var win32PhantomJSPath = function () {
// get the path stored in phantomjs\lib\location.js, someting like
// "C:\\Users\\user-name\\AppData\\Roaming\\npm\\phantomjs.CMD"
var cmd = require('phantomjs').path;
var phantomJSExePath = function () {
// If the path we're given by phantomjs is to a .cmd, it is pointing to a global copy.
// Using the cmd as the process to execute causes problems cleaning up the processes
// so we walk from the cmd to the phantomjs.exe and use that instead.

// get the global npm install directory by removing the filename from cmd variable
var npmGlobalRoot = path.dirname(cmd);
 var phantomSource = require('phantomjs').path;

// add known path
var phantom = npmGlobalRoot + '\\node_modules\\phantomjs\\bin\\phantomjs';
 if (path.extname(phantomSource).toLowerCase() === '.cmd') {
   return path.join(path.dirname( phantomSource ), '//node_modules//phantomjs//lib//phantom//phantomjs.exe');
 }

return phantom;
 return phantomSource;
};

var PhantomJSBrowser = function(baseBrowserDecorator, config, args) {
Expand Down Expand Up @@ -48,12 +47,7 @@ var PhantomJSBrowser = function(baseBrowserDecorator, config, args) {
optionsCode.join('\n') + '\npage.open("' + url + '");\n';
fs.writeFileSync(captureFile, captureCode);

var isWin = /^win/.test(process.platform);
if (isWin) {
flags = flags.concat(win32PhantomJSPath(), captureFile);
} else {
flags = flags.concat(captureFile);
}
flags = flags.concat(captureFile);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because changing the executable only needs to be done when the win32 DEFAULT_CMD is set, we no longer need to detect the os version again here.

// and start phantomjs
this._execCommand(this._getCommand(), flags);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The extra flag added here caused the command to be

> node path/to/phantomjs /path/to/capture.js

But when PHANTOMJS_BIN was set, it resulted in a command similar to:

> global/path/to/phantomjs.exe path/to/phantomjs /path/to/capture.js

Which caused phantomjs to hang.

Expand All @@ -66,7 +60,7 @@ PhantomJSBrowser.prototype = {
DEFAULT_CMD: {
linux: require('phantomjs').path,
darwin: require('phantomjs').path,
win32: process.execPath //path to node.exe, see flags in _start()
win32: phantomJSExePath()
},
ENV_CMD: 'PHANTOMJS_BIN'
};
Expand Down