567904 - Add self.addonId

Status: RESOLVED WONTFIX

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

Description

[Drew Willcoxon :adw]

Attachment: patch

self.id is the JID, but modules sometimes need the add-on ID -- simple storage for example, when it talks to the extension manager.  We should add an `addonId` to the self module.

This patch also turns self.id into a getter, since people shouldn't be able to overwrite it.  And it updates simple storage to use the new property.

Comment 1

[Brian Warner [:warner :bwarner]]

What's the difference between these two forms of id? self.id was intended to be the add-on ID, and was renamed from "jetpackID" to "addonID" to finally just "id" because there didn't seem to be any necessary qualifiers for it.

Hm, do you mean that self.id looks like "jid0-XYZ", whereas the add-on-id is like "jid0-XYZ@jetpack" (one is derived from the other by appending a suffix to make it email-like?).

Hm, in that case, yeah, I think self.id ought to be the -jetpack form: identical to what the add-on manager thinks. That would make it different than what package.json contains, which is the non-suffixed form.

Comment 2

[Drew Willcoxon :adw]

(In reply to comment #1)
> Hm, do you mean that self.id looks like "jid0-XYZ", whereas the add-on-id is
> like "jid0-XYZ@jetpack" (one is derived from the other by appending a suffix to
> make it email-like?).

Right.

> Hm, in that case, yeah, I think self.id ought to be the -jetpack form:
> identical to what the add-on manager thinks. That would make it different than
> what package.json contains, which is the non-suffixed form.

In that case would the JID be exposed at all?  I presume not.  That might be a better solution, yeah, but since simple storage is already using the JID, we'd need to switch ASAP and block 0.4 on it to avoid a transition in 0.5.

Atul?

Comment 3

[Brian Warner [:warner :bwarner]]

Hmm. Now I'm uncertain. The JID (being the shortest string around) is sort of the seed from which all these other ID strings are derived. If we only send one thing into harness-options.json, I'd like it to be the short JID, and then derive the longer things (including the addon id) as necessary. (I'm happy to see browser-side code which adds prefixes and suffixes to transform this seed into the right format, but I'd like to avoid having that code trim/remove a suffix to achieve similar things).

That makes me lean towards having "packaging.jetpackID", "self.id", and "self.addonid" . The use of the word "jetpack" in the internal-only "packaging" object feels ok. I know that we were trying to keep "jetpack" out of any of the "self.*" property names. Is the same true with the word "addon"? Will it be weird to have jetpack/addon/extension/whatever code that uses "self.addonid" when this code is not actually a FF addon?

(one reason we went with self.id is that it avoids all of these problems, but of course, There Can Be Only One, at least only one with a short name like "id"...)

Comment 4

[Atul Varma [:atul]]

I think that self.id and self.addonID are fine, personally.

Comment 5

[Brian Warner [:warner :bwarner]]

Ok, let's go with self.id = packaging.jetpackID = "jid0-XYZ", and self.addonID = packaging.addonID = "jid0-XYZ@jetpack". The diff from your patch would be to set harness_options["addonID"] = addon_id in python-lib/cuddlefish/__init__.py at line 485, and then reference packaging.addonID instead of packagind.jetpackID+"@jetpack" in self.js .

Comment 6

[Drew Willcoxon :adw]

Thanks guys.  Removing from 0.4 blocker list, since it's only cleanup and the solution doesn't impact simple storage.

Comment 7

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

I hesitate to suggest this, since the shed has already been well painted, but given that the ID in question is the one used by the Toolkit API <https://developer.mozilla.org/en/Toolkit_API> to identify the installable bundle <https://developer.mozilla.org/en/Bundles> by which an addon is installed, the most appropriate name for this would probably be bundleID.  And that name would also be compatible with uses of the SDK to build XULRunner applications.

Comment 8

[Drew Willcoxon :adw]

Atul, Brian, any response to Myk's comment 7?

Comment 9

[Brian Warner [:warner :bwarner]]

I'm ok with bundleID. I'd still put it in self.bundleID and packaging.bundleID , and continue to have the cuddlefish/__init__.py code take responsibility for computing it and storing it in harness_options[]

Comment 10

[Atul Varma [:atul]]

Oh, had no idea about the 'bundle' terminology--I'm fine with it, though we may also want to add it to our SDK documentation glossary, and possibly start using the word in contexts where it's appropriate.

Comment 11

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

(In reply to comment #10)
> Oh, had no idea about the 'bundle' terminology--I'm fine with it, though we may
> also want to add it to our SDK documentation glossary, and possibly start using
> the word in contexts where it's appropriate.

Yup, yup.  FWIW, I used to call them packages, and that's still a common way of describing them, but since we use "package" to mean something else, and we use it a lot, it would be better to use a different word here.

Comment 12

[Atul Varma [:atul]]

Comment on attachment 447228 [details] [diff] [review]
patch

Looks good, but change addonId -> bundleId?

Comment 13

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

However, nit: bundleId -> bundleID, for consistency with packaging.jetpackID as well as usage of contractID, classID, and timerID.

Comment 14

[Drew Willcoxon :adw]

Bug 549324 landed and added packaging.bundleID, per Brian's comment 9 here.  self.bundleID is no longer necessary, and adding it would be redundant, so resolving wontfix.

Comment 15

[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".