Closed Bug 653109 Opened 13 years ago Closed 13 years ago

Document new worker.port attribute

Categories

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

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ochameau, Assigned: wbamberg)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch Preliminary doc, only in API (obsolete) — Splinter Review
Now that bug 635748 landed, we need to document new EventEmitter attribute existing on all Worker objects: Panel, Page-worker and Page-mod workers.

Here is a first patch that only add document about this new port in interfaces.

Then we'll need to rethink guides:
https://jetpack.mozillalabs.com/sdk/1.0b4/docs/dev-guide/addon-development/events.html
https://jetpack.mozillalabs.com/sdk/1.0b4/docs/dev-guide/addon-development/web-content.html
And try to hide postMessage everywhere around the documentation in order to emphasis `port` and EventEmitter API.
Attachment #528585 - Flags: review?
Attachment #528585 - Flags: review? → review?(wbamberg)
Before I go ahead and update the docs I'd like to make sure I understand this change fully. I've summarized my understanding of it here; perhaps you could check that my understanding is correct? There are a couple of questions at the end.

***
Modules that use content scripts now have the ability to communicate with them using custom events, rather than simple messages.

1. Working with custom events
-----------------------------
1.1 - to send a custom event from a content script to the add-on code:

Use "self.port.emit". This takes 2 arguments: the name of the event, and the event payload:

    self.port.emit('show', content_to_show)

1.2 - to receive a custom event in the add-on code

Use the 'on' method of the 'port' property of the relevant worker object. This takes two arguments: the name of the event and a handler function.

While panel and page-worker are worker objects themselves, page-mod isn't, but passes you a worker object in the built-in 'attach' event':

    panel.port.on('show', function(content_to_show) {
      // handler
    });
...
    onAttach: function(worker) {
      worker.port.on('show', function(content_to_show) {
        // handler
      })
    }

1.3 - to send a custom event from the add-on code to the content script:

Use the 'emit' event on the 'port' property of the worker object:

    panel.port.emit('load', content_to_load)

1.4 - to receive a custom event in a content script:

Use the 'on' method of the 'port' property of the global 'self' object:

    self.port.on('load', content_to_load)

2. Working with built-in events
-------------------------------
Most Jetpack modules also include a set of built-in events. The set of built in events for a module is listed in its API docs. To receive an event from an API in your add-on code, use the corresponding object's 'on' function:

    panel.on('show')

High level APIs generally don't emit built-in events, nor do content scripts generally receive them. The exception to this is the (now deprecated) 'message' event.

3. Working with message events
------------------------------
The built-in 'message' event enables add-on code to communicate with content scripts. The syntax is:

- to send from content script, use the global postMessage function:

    postMessage('show', payload)

- to receive in add-on code, use the 'on('message')' function attached to the relevant worker:

    panel.on('message', function(message){...});

- to send from add-on code, use the worker's postMessage function:

    panel.postMessage('show', payload)

- to receive in content script code use the 'on' function attached to the global 'self' object:

    self.on('message', function(message){...});

4. Deprecation of message events
--------------------------------
Message events are now deprecated in favour of built-in events. Message events force the recipient to implement a switch statement to figure out which message is being received:

      worker.on('message', function(message) {
        switch(message.kind) {
          case 'show':
            // do one thing
            break;
          case 'hide':
            // do something else
            break;
        }

Custom events eliminate this:

      worker.port.on('show', function(data) {
        // do one thing
      });

      worker.port.on('hide', function() {
        // do something else
      });

5. Questions
------------
- what happens if I still use message events? Nothing? Warnings?
- I'm not so clear on what happens with widgets and context menus.
(In reply to comment #1)
> 1. Working with custom events
> -----------------------------
> 1.1 - to send a custom event from a content script to the add-on code:
> 
> Use "self.port.emit". This takes 2 arguments: the name of the event, and the
> event payload:
> 
>     self.port.emit('show', content_to_show)

I don't know if we want to advertise this EventEmitter feature, but emit is an n-arguments method, so we can pass as many arguments as we want; They will all be passed to the matching listener.

> 2. Working with built-in events
> -------------------------------
Here is some built-in examples:
- 'context' event in context-menu, sent to the content script,
- 'error' or 'detach' event on all workers, sent to the worker,
- 'click' event on widget, sent to the widget object

Widget doesn't compose with worker, but deleguate to a worker.
So Widget do not have port attribute right now, but I'm working on this.
(i.e. Widget is still not compatible with EventEmitter style)


> 3. Working with message events
> ------------------------------
> The built-in 'message' event enables add-on code to communicate with content
> scripts. The syntax is:
> 
> - to send from content script, use the global postMessage function:
> 
>     postMessage('show', payload)

We are going to deprecate postMessage/on globals.
So suggest using self.postMessage instead of postMessage.
You already suggested self.on instead of on.
See bug 653187 about deprecation status, it's still in process.

> 4. Deprecation of message events
> --------------------------------
> 
> 5. Questions
> ------------
> - what happens if I still use message events? Nothing? Warnings?

What's clear is that we are going to deprecate globals.
But it's not very clear to me if we will deprecate custom messages in some way:
documentation notes, console messages, ... ?
Myk: we didn't really talked about that, do you think we should start deprecating postMessage for custom messages in 1.0b5 ?

> - I'm not so clear on what happens with widgets and context menus.

For widget, as I said, the port attribute is missing on Widget class.
I'm trying to provide a patch ASAP.
About context-menu, from what I know, we just need to use self.on("context",... instead of on("context",...
Thanks Alex.

> > 2. Working with built-in events
> > -------------------------------
> Here is some built-in examples:
> - 'context' event in context-menu, sent to the content script,
> - 'error' or 'detach' event on all workers, sent to the worker,
> - 'click' event on widget, sent to the widget object

Sure, I know about these examples (well actually I had overlooked 'context', so it's not true to say, as I did above, that content scripts don't generally need to listen to built-in events). But my understanding is that the add-on developer does not write code to _emit_ these events: they are emitted by the SDK modules. So the content script doesn't emit 'click', nor does the add-on code emit 'context'. So for the high-level docs, we need to document how to respond to them but not how to emit them. Is that correct?

> > - I'm not so clear on what happens with widgets and context menus.
> 
> For widget, as I said, the port attribute is missing on Widget class.
> I'm trying to provide a patch ASAP.

Do we expect it'll land in this release, or should I document it separately?
Assignee: nobody → wbamberg
(In reply to comment #2)
> What's clear is that we are going to deprecate globals.
> But it's not very clear to me if we will deprecate custom messages in some way:
> documentation notes, console messages, ... ?
> Myk: we didn't really talked about that, do you think we should start
> deprecating postMessage for custom messages in 1.0b5 ?

No, I don't think we should deprecate postMessage.  But I do think we should emphasize the new EventEmitter API and use it in examples.
(In reply to comment #3)
> > > - I'm not so clear on what happens with widgets and context menus.
> > 
> > For widget, as I said, the port attribute is missing on Widget class.
> > I'm trying to provide a patch ASAP.
> 
> Do we expect it'll land in this release, or should I document it separately?

I tried to add `port` attribute in widget, but it's far from being easy. And as most of the code is going to change if WidgetView lands ... I'm not sure that we will have time to land this, but until we didn't ship first RC, I'll keep trying to work on this.
P1 for 1.0, although we'll take for 1.0b5 if it's ready in time.
Priority: -- → P1
Target Milestone: --- → 1.0
(In reply to comment #3)
Will: I'm finally able to provide a patch to implement Widget.port in bug 649629.
(In reply to comment #6)
> P1 for 1.0, although we'll take for 1.0b5 if it's ready in time.

Alex said the postMessage -> self.postMessage change will be documented by this bug (cf. bug 653187 comment 10).  Since that change landed for b5, it needs to be documented for b5, so at least that part of this bug (or a new spun-out bug specifically for that documentation) needs to land for b5, no?
(In reply to comment #8)
> (In reply to comment #6)
> > P1 for 1.0, although we'll take for 1.0b5 if it's ready in time.
> 
> Alex said the postMessage -> self.postMessage change will be documented by this
> bug (cf. bug 653187 comment 10).  Since that change landed for b5, it needs to
> be documented for b5, so at least that part of this bug (or a new spun-out bug
> specifically for that documentation) needs to land for b5, no?

We deprecated the global postMessage but didn't remove it, so it's not absolutely essential that the docs on the postMessage -> self.postMessage change land, since addon code that uses the old global function will continue to work.

Nevertheless, I would very much like to land those docs, as it'd be much better to let developers know about about this change sooner than later.
Attached patch monstrous patchSplinter Review
Most of the weight is a couple of new diagrams, but most of the content is in the 'content scripts guide'.

Apart from that:
- updated API reference docs for the relevant modules
- updated translator tutorial
- updated annotator tutorial to match the changes Alex made (although I also updated widget to use port)

This change includes the deprecating of global 'on', 'postMessage', although I didn't specifically say the old way is deprecated, thinking we could do that in release notes.

If you can bear to, it would probably be worth walking through the annotator tutorial checking it still works: I did do this, but it never hurts to have another pair of eyes on it.
Attachment #528585 - Attachment is obsolete: true
Attachment #528585 - Flags: review?(wbamberg)
Attachment #529252 - Flags: review?(myk)
Comment on attachment 529252 [details] [diff] [review]
monstrous patch

Review of attachment 529252 [details] [diff] [review]:

I went through the annotator tutorial, and the results were mostly as expected, except for some wonky behavior around the selection of elements to annotate (I couldn't select anything other than a large chunk of the page), and I didn't see a yellow border around that annotated element.  And I got the same results running examples/annotator/.

But I also got them running the current version of examples/annotator/ too.  So clearly the wonkiness is not a consequence of this patch.

r=myk

::: packages/addon-kit/docs/page-worker.md
@@ +107,5 @@
+<api name="port">
+@property {EventEmitter}
+[EventEmitter](packages/api-utils/docs/events.html) object that allows you to:
+* send events to the content script using the `port.emit` function
+* receive events from the content script using the `port.on`

Nit: `port.on` -> `port.on` function

Also, this is yet another case where we link from addon-kit to api-utils doc, which is problematic. We can fix this later, though. I don't have an immediate solution.

::: packages/addon-kit/docs/panel.md
@@ +128,5 @@
+<api name="port">
+@property {EventEmitter}
+[EventEmitter](packages/api-utils/docs/events.html) object that allows you to:
+* send events to the content script using the `port.emit` function
+* receive events from the content script using the `port.on`

Nit: `port.on` -> `port.on` function

::: packages/addon-kit/docs/widget.md
@@ +367,5 @@
+@property {EventEmitter}
+[EventEmitter](packages/api-utils/docs/events.html) object that allows you to:
+
+* send events to the content script using the `port.emit` function
+* receive events from the content script using the `port.on`

Nit: `port.on` -> `port.on` function

::: packages/api-utils/docs/content/worker.md
@@ +73,5 @@
+<api name="port">
+@property {EventEmitter}
+[EventEmitter](packages/api-utils/docs/events.html) object that allow to:<br>
+- send customized messages to the worker via `port.emit` function, <br>
+- observe events coming from the worker with `port.on` method.

Nit: allow to -> allows you to

Nit: both `function` and `method` are accurate descriptions of `port.emit` and `port.on`, but we should use one word consistently.

::: static-files/md/dev-guide/addon-development/web-content.md
@@ +69,4 @@
 the `contentScript` option in its constructor, and does not need to be
 maintained as a separate file at all.
 
+## Loading Content Scripts ##

Title Capitalization FTW!
Attachment #529252 - Flags: review?(myk) → review+
a=myk for commission during freeze.
Fixed by: https://github.com/mozilla/addon-sdk/commit/fa651783509bcd11127369b2a8c828b58c057018
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: