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

Bug 746131: CFX in JS - Use an SDK addon called by cfx in python that is going to install the xpi #410

Closed
wants to merge 19 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
116 changes: 116 additions & 0 deletions cfx/README.md
@@ -0,0 +1,116 @@
# cfx-js

cfx-js is an SDK addon meant to replace python command line application.
This addon is currently quite limited but it is going to implement all features
of cfx python application in order to remove python dependency.
It currently just allow to build an XPI file.
For now, it is not meant to be used standalone, it is an helper addon used by
add-on sdk `cfx` command line application.


# How does it work

* cfx python application set `CFX_OPTIONS_FILE` environnement variable to an
absolute path to a JSON file that contains necessary information to execute
some commands,
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you want to end with , here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I did. Isn't the same rule between french and english ?
When you have multiple bullets, you end up with coma for all bullets expect the last one, with a point.

* Then, cfx python application run it through the xulrunner template,
* Finally, the addon reads JSON from `CFX_OPTIONS_FILE` and performs tasks
described by it.

(We aren't putting JSON content into environnement variable as the size of an
environnement variable is quite limited)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think such implementation details needs to be in readme. If you really want it, just add "(use JSON file since options may not fit in environment variable)" at the end of first bullet (line 15).



# How to build it and install it

* Activate your SDK environnement

$ source bin/activate (linux/mac)
- or -
$ bin\activate (window)

* Go to cfx-js directory

$ cd cfx/

* Generate cfx-js xpi

$ cfx xpi

* Then, copy it to the xulrunner application template

$ mv cfx.xpi xulrunner-app/


# Expected JSON Object passed to cfx-js

{
"command": "string" // Name of the command to execute
"options": "object" // Arguments needed to execute this command
}

## Supported commands

### install-xpi

Install a given xpi into the currently running firefox/xulrunner application

Required `options` object:
{
"path": "string" // Absolute path to the xpi to install
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think more complete examples would help a lot:

{
  "command": "install-xpi",
  "options": {
    "path": "/path/to/addon.xpi"
  }
}

Maybe also note what the path format will be for windows.


### build-xpi

Build an addon SDK addon.

Required `options` object:
{
Copy link
Contributor

Choose a reason for hiding this comment

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

You're still mixing two naming conventions in the single JSON, please stick to either camel case or dash delimited naming convention. The later one seems to be more popular in JSON so I'd prefer that one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used camel case in order to be able to use destructuring and/or regular attribute access like option.xpiPath/let { xpiPath } = options;, instead of only options["xpi-path"]. Having said that I'm fine about dash-delimited too.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see why you used camel case and it's convenient, but I still think that consistency is more important. So if you don't mind let's go with either dash-delimited keys or single word names. I think you could just use xpi, template, rdf as names.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed to dash-delimited. I kept long names as they are explicit.

"xpi-path": "string" // Absolute path to the xpi we want to build
"template-path": "string" // Absolute path to the temaplate folder.
// All files from this folder are going to be
// written in the xpi
"install-rdf": "string" // Content of the `install.rdf` to write in the xpi
"harness-options": {
"jetpackID": "string" // Addon id, "some-unique-string@jetpack", or, UUID
// like "{79daaae6-5916-49ba-8d3c-f54df65f210b}"
"icon": "string" // Absolute path the the default icon for this addon
"icon64": "string" // Same thing, but for a 64px version of it
"packages": { // A dictionnary of packages. keys are packages names
// Values are another dictionnary with packages section
// folders. These folders are written into `resources`
// directory in the xpi
"api-utils": { // package name
"test": section, // section name => absolute path to it
"lib": section
}
},
"locale": { // A dictionnary of available locales for this addon
// keys are language code and values are another dictionnary
// with all translated strings
"en-US": {// language code
"key": "translation" // key to translate => translated key
}
},
"limitTo": [ // Optional list of white-listed files to accept into the xpi
// Used to select which module will be copied
"/sdk/addon-kit/lib/page-mod.js",
"..."
],
"preferences": { // Optional dictionnary of preferences fields supported
// by this addon
{ "type": "string" // Preference type, can be string, bool
// or integer,
"name": "stringPreference", // Preference internal name,
"value": string, bool or integer, // Preference default value,
"title": "string" // Preference title.
},
...
}
// All these `harness-options` attributes are used to build the xpi
// but you can pass other attributes. They will be written into
// `harness-options.json` at xpi's root folder
},
"extra-harness-options": "dictionnary" // A set of additional attributes to
// write into `harness-options.json`
}
72 changes: 72 additions & 0 deletions cfx/cfx.js
@@ -0,0 +1,72 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */
"use strict";

const { Cc, Ci } = require("chrome");
const AddonInstaller = require("api-utils/addon/installer");
const file = require("api-utils/file");
const system = require("api-utils/system");
const xpi = require("./xpi");

function getOptions() {
let optionsFile = system.env["CFX_OPTIONS_FILE"];
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Why not just system.env.CFX_OPTIONS_FILE ?

if (!optionsFile) {
throw new Error("Unable to locate options file, " +
"environment variable is not set.");
}
let data = null;
try {
data = file.read(optionsFile);
}
catch(e) {
throw Error("Unable to read options file: " + optionsFile + "\n" + e);
}
return JSON.parse(data);
}

const COMMANDS = {
"install-xpi": function (options) {
AddonInstaller.install(options.path)
.then(
null,
function onError(error) {
console.log("Failed to install addon: " + error);
});
},
"build-xpi": function (options) {
xpi.build(options);
},
"no-quit": function (options) {
// Test command in order to simulate a run that never quits
Cc['@mozilla.org/toolkit/app-startup;1'].
getService(Ci.nsIAppStartup).
enterLastWindowClosingSurvivalArea();
}
};

function main() {
try {
let { command, options } = getOptions();
if (command in COMMANDS)
COMMANDS[command](options);
else
console.log("Unknown cfxjs command '" + command + "'");
}
catch(e) {
if ("cfxError" in e) {
// Stacktrace isn't usefull in case of custom cfx exceptions, prints
// custom message instead. These exceptions are implemented in
// exception module.
dump(e.toString() + "\n");
}
else {
console.error("Unknown internal error in cfx.js:");
console.exception(e);
}
system.exit(system.E_FORCE);
}
}

if (require.main === module)
main();
25 changes: 25 additions & 0 deletions cfx/exception.js
@@ -0,0 +1,25 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */
"use strict";

// Set of custom exception class for cfx.js
Copy link
Contributor

Choose a reason for hiding this comment

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

Please "use strict"

// Designed to be handled nicely from main module

function cfxError(message, name) {
let error = Error(message);
error.cfxError = true;
error.name = name;
error.toString = function () {
return this.name + ": "+ this.message;
};
return error;
}

exports.InvalidArgument = function InvalidArgument(message) {
return cfxError(message, 'InvalidArgument')
}

exports.InternalCfxError = function InternalCfxError(message) {
return cfxError(message, 'InternalCfxError')
}
9 changes: 9 additions & 0 deletions cfx/package.json
@@ -0,0 +1,9 @@
{
"id": "cfx",
"name": "cfx-js",
"description": "Helper addon for cfx to build xpi files for addon sdk addons",
"main": "./cfx.js",
"directories": {
"lib": "."
}
}
109 changes: 109 additions & 0 deletions cfx/preferences.js
@@ -0,0 +1,109 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */
"use strict";

const { Cc, Ci } = require("chrome");
Copy link
Contributor

Choose a reason for hiding this comment

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

license block + "use strict"

const DOMSerializer = Cc["@mozilla.org/xmlextras/xmlserializer;1"].
createInstance(Ci.nsIDOMSerializer);

const { InvalidArgument } = require("./exception");

const VALID_PREF_TYPES = ['bool', 'boolint', 'integer', 'string', 'color',
'file', 'directory', 'control'];

// Validate preferences from `preferences`'s package.json attribute
function validate(options) {
for (let key in options) {
let pref = options[key];
// Make sure there is a 'title'
if (!("title" in pref))
throw new InvalidArgument("The '" + pref.name + "' pref requires a 'title'");

// Make sure that the pref type is a valid inline pref type
if (VALID_PREF_TYPES.indexOf(pref.type) === -1)
throw new InvalidArgument(pref.type + " is not a valid inline pref type");

// Make sure the 'control' type has a 'label'
if (pref.type == "control" && !("label" in pref))
throw new InvalidArgument("The 'control' inline pref type requires a 'label'");

// TODO: Check that pref["type"] matches default value type
Copy link
Contributor

Choose a reason for hiding this comment

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

Wanna subbmit a follow-up bug for this and put a link here, to make sure we do that at some point ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filled: https://bugzilla.mozilla.org/show_bug.cgi?id=772126
This TODO comes from python code. Good candidate for a follow-up!

}
}
exports.validate = validate;

function createXULDocument(content) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think nsIDOMParser is the right way to go about this.

let xhr = Cc["@mozilla.org/xmlextras/xmlhttprequest;1"].
createInstance(Ci.nsIXMLHttpRequest);
xhr.open("GET", "data:application/vnd.mozilla.xul+xml," +
"<?xml version=\"1.0\"?>" + content, false);
xhr.send(null);
return xhr.responseXML;
}

// Takes preferences`'s package.json attribute and returns the options.xul
// file content needed to build preference panel opened from about:addons
function generateOptionsXul(options, jetpackId) {
let xulns = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
let content = "<vbox xmlns=\"" + xulns + "\"></vbox>";
let doc = createXULDocument(content);
let root = doc.documentElement;

for (let key in options) {
let pref = options[key];
let setting = doc.createElement("setting");
setting.setAttribute("pref", "extensions." + jetpackId + "." + pref.name);
setting.setAttribute("type", pref.type);
setting.setAttribute("title", pref.title);

if ("description" in pref)
setting.appendChild(doc.createTextNode(pref.description));

if (pref.type == "control") {
button = doc.createElement("button");
button.setAttribute("label", pref.label);
button.setAttribute("oncommand", "Services.obs.notifyObservers(null, '" +
jetpackId + "-cmdPressed', '" +
pref.name + "');");
setting.appendChild(button);
}
else if (pref.type == "boolint") {
setting.setAttribute("on", pref.on);
setting.setAttribute("off", pref.off);
}
root.appendChild(setting);
}

return DOMSerializer.serializeToString(root);
}
exports.generateOptionsXul = generateOptionsXul;

function isFloat(value) {
return typeof(value) == "number" && value % 1 !== 0;
}

/**
* Based on preferences manifest written in package.json file of an addon,
* returns the necessary prefs.js file content. This file is going to set
* default preference values when the addon will be installed.
*/
function generatePrefsJS(options, jetpackId) {
let prefList = []

for (let key in options) {
let pref = options[key];
if (!('value' in pref))
continue;
// TODO: Understand why python code ignore float and only floats?
Copy link
Contributor

Choose a reason for hiding this comment

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

the only acceptable types are bool, int, and string. So that is why floats were ignored. It might be better to throw a warning, or error here tho, and the other acceptable JSON types (ie: object, array, and null) should be treated the same as a float.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated this patch again in order to accept only bool, int or string and throw an error otherwise.
Feel free to tell me if there is still something wrong.

if (isFloat(pref.value))
continue;

let prefKey = "extensions." + jetpackId + "." + pref.name;
let prefValue = JSON.stringify(pref.value);
prefList.push("pref(\"" + prefKey +"\", " + prefValue + ");");
}

return prefList.join("\n") + "\n";
}
exports.generatePrefsJS = generatePrefsJS;