610816 - Store options.staticArgs as an object instead of a serialized JSON string

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect)

Description

[Atul Varma [:atul]]

The Python cuddlefish package seems to set 'staticArgs' in harness-options.json to the *stringified* version of the '--static-args' option passed to cfx rather than the parsed version, which results in options.staticArgs being a string instead of a JSON object.

Drew, was this intentional, or is it a bug?

Comment 1

[Myk Melez [:myk] [@mykmelez]]

The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product.

To filter bugmail related to this change, filter on the word "looptid".

Comment 2

[Drew Willcoxon :adw]

bug 599181 comment 6

Comment 3

[Atul Varma [:atul]]

Ah, yeah, I'd forgotten that discussion... While I don't care how it's done within the implementation of cfx for the purposes of this discussion, the problem this bug focuses on is the fact that the example the docs mention [1] doesn't work:

  exports.main = function (options, callbacks) {
    console.log(options.staticArgs.foo);
  };

In the current codebase, undefined would be logged to the console because options.staticArgs is a string. You'd have to do console.log(JSON.parse(options.staticArgs).foo) to get the desired result... We can do the parsing of the JSON either in cfx, when we write out the object to harness-options.json, or we can do it in the actual components/harness.js file that constitutes the "bootloader" of each Jetpack-based XPI, but it seems like either way, we should be parsing the string for the main() function so it doesn't have to do it itself... 

[1] https://jetpack.mozillalabs.com/sdk/0.9/docs/#guide/cfx-tool

Comment 4

[Drew Willcoxon :adw]

What?  Something regressed that behavior then, because it worked when I added it.

Comment 5

[Drew Willcoxon :adw]

https://github.com/mozilla/addon-sdk/blob/master/python-lib/cuddlefish/app-extension/components/harness.js#L525

Comment 6

[Drew Willcoxon :adw]

Atul, can you still reproduce this?  I can't (with commit 0295a0ea442e59f6ba2b7bd8f3d68263d513d656).

Comment 7

[Atul Varma [:atul]]

Oh, crud. It looks like this is actually causing problems with 'cfx develop' and possibly other things that run harness.js without actually calling its getDefaults() function. This means that anything else that wants to directly call harness.js's buildHarnessService() without calling getDefaults() would need to manually JSONify options.staticArgs first.

I think one option is to parse out the JSON blob in buildHarnessService() instead of getDefaults(); however, even doing that would require that other tools that examine the harness-options.json blob outside of the addon, such as warner's manifest-inspection tool, be cognizant about deserializing options.staticArgs as well.

In other words, seems like it'd be a lot easier for most stakeholders to just assume that options.staticArgs is a JSON object, rather than a JSON-serialized string. So do you think we could do the JSON deserialization in cfx rather than requiring all code that reads harness-options to manually deserialize it?

Comment 8

[Drew Willcoxon :adw]

I don't care.

Comment 9

[Atul Varma [:atul]]

Attachment: Pointer to pull request

Comment 10

[Atul Varma [:atul]]

Comment on attachment 492989 [details]
Pointer to pull request

Cool, can you review this patch then?

Comment 11

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 492989 [details]
Pointer to pull request

This seems low-risk and useful to land before the first beta. a=myk

Comment 12

[Atul Varma [:atul]]

Pushed:

https://github.com/mozilla/addon-sdk/commit/6252f40f49ae0090b495f6f7487ad6a1337fb34e