635557 - Create a guide to help transition current add-on devs to the SDK

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: Documentation, defect, P1)

Description

[Drew Willcoxon :adw]

On the mailing list, pd says [1] there should be a guide for current add-on devs to help transition them from XUL and all the things involved in traditional add-on development to the SDK.  That's a good idea.  I guess it would basically show them how to translate common patterns and practices in traditional extensions to the SDK.

[1] http://groups.google.com/group/mozilla-labs-jetpack/browse_thread/thread/d14fa11ff1b68957

Comment 1

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Is there a more current bug tracking this work? I could've sworn I've seen one, but a quick search isn't giving me anything. (Maybe I'm thinking of the new Feature page for it?)

Comment 2

[Will Bamberg [:wbamberg]]

No, this is the right bug for this, I think.

Comment 3

[Will Bamberg [:wbamberg]]

Also see bug 663541, which explains how we expect developers to port code that uses Components.

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(Pushing all open bugs to the --- milestone for the new triage system)

Comment 5

[Will Bamberg [:wbamberg]]

Attachment: XUL migration guide, finally

Finally, here's a patch for the XUL migration guide. There are three new pages:

* a short tutorial on using third party modules
* a main XUL migration page
* a page listing benefits and limitations of the SDK for XUL developers (note: for XUL developers, so benefits like simplicity and packaging support aren't listed)

I've gone back and forth a few times over the structure, what should be split into separate pages, and whether it really hang together as a guide rather than a collection of potentially useful information, and so on. But I figure this is good enough as a basis for negotiation, anyway.

Some random thoughts:

I think eventually we should have a more complex example which uses low-level modules, and at that point we should split the library detector example out into its own page, and just point at it from the migration guide.

My original version of the library detector example was much more detailed, going through the code, but I decided this was boring and not very useful, and it was better to highlight the overall design and point at the code.

I didn't ask Erik Vold yet whether he's OK with using menuitems as an example 3rd party module. A problem with that is that it's a moving target, which we don't control. Maybe it would be good enough to point to, or clone, a specific revision of menuitems, as we do when we point to examples in the Builder?

Comment 6

[Will Bamberg [:wbamberg]]

Attachment: Fixed a copule of typos

Sorry! Same patch, with a couple of typos fixed. If you prefer, you can find the change at: https://github.com/wbamberg/jetpack-sdk/tree/migration-guide.

Comment 7

[Drew Willcoxon :adw]

Comment on attachment 566879 [details] [diff] [review]
Fixed a copule of typos

Review of attachment 566879 [details] [diff] [review]:
-----------------------------------------------------------------

::: doc/dev-guide-source/addon-development/sdk-vs-xul.md
@@ +1,3 @@
> +
> +# Benefits of the SDK for XUL Developers #
> +

There are other advantages relevant to XUL developers:

* Less code for you to write, since you can rely on our APIs, which means you can turn out a new add-on much more quickly, and the code you do write is likely to have less bugs and will be easier to maintain.

* By using our APIs your add-on is automatically bestowed with UI and UX best practices, which means a better, more consistent experience for your users.

::: doc/dev-guide-source/addon-development/third-party-modules.md
@@ +6,5 @@
> +
> +In this example we'll use Erik Vold's
> +[`menuitems`](https://github.com/erikvold/menuitems-jplib) module to add a menu
> +item to Firefox's Tools menu.
> +

The example is nice, but I'd like to see a concise description of what I need to do to use third-party packages in my add-on before reinforcing that by reading the example.

@@ +12,5 @@
> +
> +First we'll download `menuitems` from
> +[https://github.com/erikvold/menuitems-jplib](https://github.com/erikvold/menuitems-jplib). Like [addon-kit](packages/addon-kit/addon-kit.html) and
> +[api-utils](packages/api-utils/api-utils.html), it's a
> +[CommonJS package](dev-guide/addon-development/commonjs.html),

What does "it" refer to, the "menuitems" module or the "menuitems-jplib" package?  This page kind of glosses over the differences between "module" and "package", but the distinction is important.  I'm not saying every page in the docs has to explain the difference, but every page should use those words consistently and correctly.

I think this page should be titled "Third-Party Packages", since it explains how to install and use packages.  I guess it's possible to "install" modules by downloading single JS files and adding them to existing packages, but the package is the atomic unit of code sharing, not the module.

@@ +53,5 @@
> +* `menuid`: identifier for the item's parent element
> +* `insertbefore`: identifier for the item before which we want our item to
> +appear
> +
> +Create a new directory and run `cfx init` in it. Open `lib/main.js` and

Where should I create this directory?

@@ +99,5 @@
> +* It's not always obvious where to find third-party modules, although some
> +are collected in the [Jetpack Wiki](https://wiki.mozilla.org/Jetpack/Modules).
> +* Third party modules typically require high security privileges, which
> +increases the damage a malicious web site could do if it were able to
> +compromise your add-on.

As someone who's worked on the SDK, I'm not sure what this means.  If I were new to the SDK, I wouldn't know what it means either, but it would scare me without giving me any information on where I could learn more about security privileges and preventing malicious web sites from harming my add-on's users.

::: doc/dev-guide-source/addon-development/xul-migration.md
@@ +72,5 @@
> +There are two related reasons for this design. The first is security: it
> +reduces the risk that a malicious web page will be able to access privileged
> +APIs. The second is the need to be compatible with the multi-process architecture
> +planned for Firefox: after this is implemented in Firefox, all add-ons will
> +need to use a similar pattern.

"... , so it's likely your add-on will need to be rewritten anyway."

@@ +77,5 @@
> +
> +There's much more information on content scripts in the
> +[Working With Content Scripts](dev-guide/addon-development/web-content.html) guide.
> +
> +## Using the "supported" APIs ##

Why's "supported" in quotes?  Also, some of the headings like this one aren't in title case, but others are.

@@ +79,5 @@
> +[Working With Content Scripts](dev-guide/addon-development/web-content.html) guide.
> +
> +## Using the "supported" APIs ##
> +
> +See this

I'm a XUL add-on developer looking at this page to see if it's worth my time to migrate.  This page is my first impression of the SDK.  I need a brief description of what "supported APIs" means before you send me off to look at another page.

@@ +112,5 @@
> +of XPCOM functionality.
> +
> +## Using Third Party Modules ##
> +
> +See the

Again, briefly tell me what third-party modules are before sending me someplace else.

@@ +126,5 @@
> +
> +If you can't find a suitable third party module, you can use low-level APIs to:
> +
> +* load and access any XPCOM component
> +* modify the browser chrome using dynamic manipulation of the XUL

"dynamic manipulation of the DOM", not XUL.

@@ +131,5 @@
> +* directly access the [tabbrowser](https://developer.mozilla.org/en/XUL/tabbrowser)
> +object
> +
> +All these techniques involve the use of low-level APIs, which don't have
> +the same compatibility guarantees as the supported APIs.

What are those guarantees?

@@ +187,5 @@
> +One of the benefits of this is that we can control which parts of the add-on
> +are granted chrome privileges, making it easier to review and secure the code.
> +
> +**If a module which uses `require("chrome")`
> +is compromised, the attacker gets full access to the browser's capabilities.**

I'm new to the SDK.  This scares me.  I don't know much about modules.  How can my module be compromised?  How can I protect it from being compromised?  etc.

@@ +192,5 @@
> +
> +### window-utils ###
> +
> +The [`window-utils`](packages/api-utils/docs/window-utils.html) module gives
> +you direct access to the browser chrome.

"direct access to the browser chrome" -> "access to chrome windows, including the browser's chrome window."

@@ +211,5 @@
> +
> +This example just removes the 'forward' button from the browser. It constructs
> +a `WindowTracker` object and assigns a function to the constructor's `onTrack`
> +option. This function will be called whenever a window is opened. The function
> +checks whether the window is the browser's XUL, and if it is, uses

"is the browser's XUL" -> "is the browser chrome window"

@@ +309,5 @@
> +Once the list is built, the `switchLibraries` function constructs a XUL
> +`statusbarpanel` element for each library it found, populates it with the
> +icon at the corresponding `chrome://` URL, and adds it to the box.
> +
> +Finally, it listen to gBrowser's `TabSelect` event, to update the contents

listen -> listens

Comment 8

[Will Bamberg [:wbamberg]]

Hey Drew

Thanks for the detailed comments. I didn't ask you to review it, only because I'm not sure how much time you have for Jetpack things these days, but since you've already looked at it in some detail, maybe I'll address those comments you have and assign it back to you to review, if you're OK with that?

A couple of things.

> * Less code for you to write, since you can rely on our APIs, which means
> you can turn out a new add-on much more quickly, and the code you do write
> is likely to have less bugs and will be easier to maintain.

For someone experienced with traditional add-on development I'd say this is debatable. If I'm writing a traditional add-on I don't have to deal with content scripts, for example, so in some cases I can write shorter and even simpler add-ons.
 
> @@ +99,5 @@
> > +* It's not always obvious where to find third-party modules, although some
> > +are collected in the [Jetpack Wiki](https://wiki.mozilla.org/Jetpack/Modules).
> > +* Third party modules typically require high security privileges, which
> > +increases the damage a malicious web site could do if it were able to
> > +compromise your add-on.
> 
> As someone who's worked on the SDK, I'm not sure what this means.  If I were
> new to the SDK, I wouldn't know what it means either, but it would scare me
> without giving me any information on where I could learn more about security
> privileges and preventing malicious web sites from harming my add-on's users.

I've been thinking for a while that we need some docs on SDK security mechanisms. I tried to write up something on it as the add-ons blog: http://blog.mozilla.com/addons/2011/10/14/security-mechanisms-in-the-add-on-sdk/ and will probably try to turn that into a guide in the docs, maybe I could then link to the relevant pieces of it from here? But I'd prefer that that didn't block this, so maybe I'll try to find something suitably concise to put here.

Comment 9

[Drew Willcoxon :adw]

(In reply to Will Bamberg [:wbamberg] from comment #8)
> if you're OK with that?

Sure, no problem.  Myk's input would be really great though, better than mine.

> For someone experienced with traditional add-on development I'd say this is
> debatable.

Hehe, fair enough. :)

> But I'd prefer that that didn't block this, so maybe I'll try
> to find something suitably concise to put here.

I think that until we have a security story to tell -- in the docs, not on a blog that people may or may not find and that may or may not be authoritative -- the docs ought not mention security one way or the other.  So I agree about not blocking this on security, but IMO rather than rushing the story we shouldn't talk about it at all.

Comment 10

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

Comment on attachment 566879 [details] [diff] [review]
Fixed a copule of typos

(In reply to Will Bamberg [:wbamberg] from comment #5)
> (note: for XUL developers, so benefits like simplicity and packaging support
> aren't listed)

These are wins for XUL developers too.  Everyone benefits from easier ways to do things, and most people prefer it, even folks who know how to do them the hard way.  That's especially true about packaging, which has traditionally been a real PITA.


diff --git a/doc/dev-guide-source/addon-development/third-party-modules.md b/doc/dev-guide-source/addon-development/third-party-modules.md

+This tells us that we need to find the `vold-utils` package and install it.
+We find that [here](https://github.com/erikvold/vold-utils-jplib),
+download it, and add it under the `packages` directory alongside `menuitems`.

This says we "need to find" the package but doesn't explain how to find it. Of course there's no good way to find it at the moment, since we don't have a central package repository. So I would leave out the part about needing to find it and gloss over that detail. Also, avoid linking just the word "here", which doesn't provide much information about the link and is a relatively small click target (so it takes extra effort to point the mouse at it).

Thus say something like this instead:

  This tells us that we need to install the `vold-utils` package,
  which we can do by downloading it from [its source repository](https://github.com/erikvold/vold-utils-jplib)
  and adding it under the `packages` directory alongside `menuitems`.

See also <http://www.w3.org/QA/Tips/noClickHere>, although I disagree with it about verbal phrases (in some cases, anyway).

Note also that my personal preference is to exclude articles from links but include possessive adjectives, so "the [angels] want to wear [my red shoes]."


+* Third party modules typically require high security privileges, which
+increases the damage a malicious web site could do if it were able to
+compromise your add-on.

This is not so much a sign of immaturity as inherent to the model.


diff --git a/doc/dev-guide-source/addon-development/xul-migration.md b/doc/dev-guide-source/addon-development/xul-migration.md

+See this [guide to the benefits and limitations of SDK development
+compared to XUL development](dev-guide/addon-development/sdk-vs-xul.html).

It felt strange to be referred to another guide that sounds like it has similar content when I'd just started reading this one.  I'd make the other page be a "chart" or "comparison" instead.


+* If your add-on needs a lot of help from the low-level APIs, then you
+won't see much benefit from migrating.

I would say rather that the cost of migrating is high and may not be worth it at this point, since you do still see a variety of benefits.


+* widgets always appear by default in the
+[add-on bar](https://developer.mozilla.org/en/The_add-on_bar),
+although users may relocate them by
+[toolbar customization](http://support.mozilla.com/en-US/kb/how-do-i-customize-toolbars)
+* there's currently no way to add items to the browser's main menus using the
+SDK's supported APIs.

Other lists on this page eschew trailing punctuation, even for the last item, so this should too for consistency.


+These are intentional design choices, the belief being that it makes for a
+better user experience for add-ons to expose their interfaces in a consistent
+way. So it's worth considering changing your user interface to align with the
+SDK APIs.

The first one is an intentional design choice; the second is just something we haven't implemented.

However, if/when we do implement it, we will probably do so by intentionally choosing to design a place in the menu system where such items can go rather than letting developers place them anywhere.


+## Using the "low-level" APIs ##
+
+If you can't find a suitable third party module, you can use low-level APIs to:
+
+* load and access any XPCOM component
+* modify the browser chrome using dynamic manipulation of the XUL
+* directly access the [tabbrowser](https://developer.mozilla.org/en/XUL/tabbrowser)
+object
+
+All these techniques involve the use of low-level APIs, which don't have
+the same compatibility guarantees as the supported APIs.
+
+### Using XPCOM ###

The document treats XPCOM and low-level modules as variants of low-level API access, but I think they're actually quite different, since low-level modules give you a powerful but simpler and limited (in the sense of exposing only a subset of the platform's functionality, not in the sense of limiting privilege) CommonJS module interface to low-level functionality, while XPCOM (in the form of require('chrome')) gives you powerful and complete access to the platform via a very different interface.

So I would make the second-level 'Using the "low-level" APIs' section be about accessing modules like `window-utils` and `tab-browser` and then have another second-level section for Using XPCOM.  Also, I'd add an explanation about the nature of the low-level APIs to its section.  And I'd put Using XPCOM after 'Using the "low-level" APIs', so the sections are ordered by preference (High-Level, Third-Party, Low-Level, XPCOM).


+You can browse and run the ported version in
+[the Builder](https://builder.addons.mozilla.org/addon/1020373/revision/65/).

Because this is the first reference to Add-on Builder in this document, use its full name.


+The XUL library detector displayed the detailed information about each
+library on mouseover in a tooltip: we can't do this using a widget, so
+instead will use a panel. We will need another content script in the
+widget which listens for icon mouseover events and sends a message to
+`main.js` containing the name of the corresponding library. `main.js`
+handles this message by forwarding the library information on to the panel,
+which updates its content.
+
+<img class="image-center" src="static-files/media/librarydetector/panel-content.png" alt="Updating panel content" />

This page ends a bit abruptly, although I'm not sure what to add.

Comment 11

[Will Bamberg [:wbamberg]]

Attachment: Reworked patch

Quite a few changes. It didn't feel right to take out any references to security in the end, so I opted to keep notes in 'using low-level modules' and 'using XPCOM', along with a more of an explanation of the rationale. We still need a security guide to link to.

I added some more detail to the example, and in consequence split it out into its own page.

> +The XUL library detector displayed the detailed information about each
> +library on mouseover in a tooltip: we can't do this using a widget, so
> +instead will use a panel. We will need another content script in the
> +widget which listens for icon mouseover events and sends a message to
> +`main.js` containing the name of the corresponding library. `main.js`
> +handles this message by forwarding the library information on to the panel,
> +which updates its content.
> +
> +<img class="image-center"
> src="static-files/media/librarydetector/panel-content.png" alt="Updating
> panel content" />
> 
> This page ends a bit abruptly, although I'm not sure what to add.

I just added a screenshot of the new add-on. Not great, but better than nothing perhaps. I thought about including a summary of what worked/what didn't, as the original blog post did: http://blog.mozilla.com/addons/2011/07/06/porting-the-library-detector-to-the-add-on-sdk/ but decided that was too specific to be useful.

A couple of things I'm unsure about. First, linking to menuitems is dodgy, since it  will change and the tutorial will become inaccurate: perhaps linking to a specific commit would be good enough? And I need to check with Erik that he's OK with using menuitems here.

Second, I'm linking to the ported library detector in the Builder, although over in bug 696699 I decided that was a Bad Idea. Still, in this case I'm not sure what's worse, linking to an Builder-hosted example, or incorporating the port into the SDK, under the examples directory. What are your thoughts on that?

Comment 12

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

Comment on attachment 572699 [details] [diff] [review]
Reworked patch

Close!

(In reply to Will Bamberg [:wbamberg] from comment #11)

> A couple of things I'm unsure about. First, linking to menuitems is dodgy,
> since it  will change and the tutorial will become inaccurate: perhaps
> linking to a specific commit would be good enough? And I need to check with
> Erik that he's OK with using menuitems here.

Linking to a specific commit seems fine.


> Second, I'm linking to the ported library detector in the Builder, although
> over in bug 696699 I decided that was a Bad Idea. Still, in this case I'm
> not sure what's worse, linking to an Builder-hosted example, or
> incorporating the port into the SDK, under the examples directory. What are
> your thoughts on that?

Provided its license is compatible, its code is in reasonable shape, and we give credit where it's due, I think it would be great to incorporate it into the SDK as an example.


diff --git a/doc/dev-guide-source/addon-development/library-detector.md b/doc/dev-guide-source/addon-development/library-detector.md

+<img class="image-right" src="static-files/media/librarydetector/library-detector.png" alt="Library Detector Screenshot" />

This image could use some margin. Right now it butts up against the text next to it.

Also, it seems like almost exactly the same image as the one at the bottom of the page, which perplexes me.  Is this one from the original version of the addon?  If so, it'd be handy to label it.  Or perhaps you could put both screenshots at the bottom of the page for a before/after effect.


+The add-on is Paul Bakaus's
+[Library Detector](https://addons.mozilla.org/de/firefox/addon/library-detector/).

The en-US version of this page seems like a better bet:

https://addons.mozilla.org/en-US/firefox/addon/library-detector/


diff --git a/doc/dev-guide-source/addon-development/xul-migration.md b/doc/dev-guide-source/addon-development/xul-migration.md

+* If your add-on needs a lot of help from the low-level APIs, then the
+cost of migrating is high, and may not be worth it at this point.
+
+* If your add-on needs a fairly limited amount of help from low-level
+APIs, then it might still be worth migrating: we'll add more supported
+APIs in future releases to meet important use cases, and eventually hope
+to have a comprehensive collection of third party modules filling many of
+the gaps.

This text still classifies everything that isn't supported as a "low-level API", whereas the rest of the document now talks about going "beyond the supported APIs" to third-party packages, low-level APIs, and XPCOM; so this should be updated to match.


+But note that unlike the supported APIs, low-level APIs do not come with a
+compatibility guarantee, so we do not expect code using them will necessarily
+continue to work as new versions of Firefox are released.

Notes like this should be more prominent, like the notes on MDN, which have a yellow background color.  Can we do something like that here, or put this on the side?


+    var windowUtils = require("window-utils");
+
+    windowUtils = new windowUtils.WindowTracker({
+      onTrack: function (window) {
+        if ("chrome://browser/content/browser.xul" != window.location) return;
+        var forward = window.document.getElementById('forward-button');
+        var parent = window.document.getElementById('unified-back-forward-button');
+        parent.removeChild(forward);
+      }
+    });

In other docs, we use `var` because it's more familiar to web developers than `let`, but in this doc, I wonder if we should use `let`, as traditional addon developers are probably familiar enough with it.

Comment 13

[Will Bamberg [:wbamberg]]

Attachment: another patch

> > Second, I'm linking to the ported library detector in the Builder, although
> > over in bug 696699 I decided that was a Bad Idea. Still, in this case I'm
> > not sure what's worse, linking to an Builder-hosted example, or
> > incorporating the port into the SDK, under the examples directory. What are
> > your thoughts on that?
> 
> Provided its license is compatible, its code is in reasonable shape, and we
> give credit where it's due, I think it would be great to incorporate it into
> the SDK as an example.

It's MIT and in fine shape, I think. I've added it, though reduced the number of libraries it covers, mostly to reduce the SDK size.

> +<img class="image-right"
> src="static-files/media/librarydetector/library-detector.png" alt="Library
> Detector Screenshot" />
> 
> This image could use some margin. Right now it butts up against the text
> next to it.
> 
> Also, it seems like almost exactly the same image as the one at the bottom
> of the page, which perplexes me.  Is this one from the original version of
> the addon?  If so, it'd be handy to label it.  Or perhaps you could put both
> screenshots at the bottom of the page for a before/after effect.

Yes, that's the 'before' version. I've ended up just taking the one at the bottom out. I only added it because (as you said) the previous version ended abruptly, but I was never very happy with it. I think it's perhaps better now that the example is in its own page and not a section of a bigger doc.

I've left the 'before' picture in because it helps with the description of what the add-on does, and breaks up the text.

> +But note that unlike the supported APIs, low-level APIs do not come with a
> +compatibility guarantee, so we do not expect code using them will
> necessarily
> +continue to work as new versions of Firefox are released.
> 
> Notes like this should be more prominent, like the notes on MDN, which have
> a yellow background color.  Can we do something like that here, or put this
> on the side?

I've made it an aside. I like asides.

> 
> +    var windowUtils = require("window-utils");
> +
> +    windowUtils = new windowUtils.WindowTracker({
> +      onTrack: function (window) {
> +        if ("chrome://browser/content/browser.xul" != window.location)
> return;
> +        var forward = window.document.getElementById('forward-button');
> +        var parent =
> window.document.getElementById('unified-back-forward-button');
> +        parent.removeChild(forward);
> +      }
> +    });
> 
> In other docs, we use `var` because it's more familiar to web developers
> than `let`, but in this doc, I wonder if we should use `let`, as traditional
> addon developers are probably familiar enough with it.

I thought it best to be consistent with the other docs, so I haven't made this change in the new patch. But of course, it's easily done.

Comment 14

[Will Bamberg [:wbamberg]]

Attachment: updated unit test

Sorry. This patch includes the update to the unit test, which is needed now that the doc-regeneration stuff pays attention to changes in base.html.

Comment 15

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

Comment on attachment 573362 [details] [diff] [review]
updated unit test

>diff --git a/examples/library-detector-sdk/README.md b/examples/library-detector-sdk/README.md

Nit: the "-sdk" suffix seems strange here.  I think I understand why you have it, but I suspect it will just confuse users, who will wonder why the other examples don't have it.  So I would leave it off.


>+It only recognizes a subset of the libraries recognized by the original,
>+just to reduce the size of the add-on.

Nit: this doesn't explain why the size matters in this case.  Perhaps add something like "... to keep the SDK download package size and on-disk footprint as small as possible."

Comment 16

[Will Bamberg [:wbamberg]]

Thanks Myk!
-> https://github.com/mozilla/addon-sdk/commit/2710fefcc657bdf64fdebdd1974f493607337d8d

Comment 17

[[github robot]]

Commit pushed to https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/c3d77a48ca65c3789ce3f7e130837dea54f928bc
Bug 635557 - Create a guide to help transition current add-on devs to the SDK; r=@mykmelez
(cherry picked from commit 2710fefcc657bdf64fdebdd1974f493607337d8d)

Comment 18

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

https://github.com/mozilla/addon-sdk/commit/c3d77a48ca65c3789ce3f7e130837dea54f928bc