Pull request 118: Widget persistent on toolbars and position r=dietri…

…ch api+r=myk

examples/annotator/lib/main.js

@@ -96,6 +96,7 @@ The selector is switched on/off with a left-click, and the list of annotations
 is displayed on a right-click.
 */
   var widget = widgets.Widget({
+    id: 'toggle-switch',
     label: 'Annotator',
     contentURL: data.url('widget/pencil-off.png'),
     contentScriptWhen: 'ready',

examples/reading-data/lib/main.js

@@ -27,6 +27,7 @@ exports.main = function(options, callbacks) {
   // Create a widget that displays the image.  We'll attach the panel to it.
   // When you click the widget, the panel will pop up.
   widgets.Widget({
+    id: "test-widget",
     label: "Mom",
     contentURL: iconURL,
     panel: myPanel

examples/reddit-panel/lib/main.js

@@ -3,6 +3,7 @@ const data = require("self").data;
 
 exports.main = function(options, callbacks) {
   widgets.Widget({
+    id: "show-reddit-btn",
     label: "Reddit",
     contentURL: "http://www.reddit.com/static/favicon.ico",
     panel: require("panel").Panel({

packages/addon-kit/docs/widget.md

@@ -30,6 +30,7 @@ using a URL with the `contentURL` property.
 For example, this widget contains an image, so it looks like a simple icon:
 
     require("widget").Widget({
+      id: "mozilla-icon", 
       label: "My Mozilla Widget",
       contentURL: "http://www.mozilla.org/favicon.ico"
     });
@@ -39,6 +40,7 @@ Upon creation, the widget is automatically added to the add-on bar.
 This widget contains an entire web page:
 
     require("widget").Widget({
+      id: "hello-display",
       label: "My Hello Widget",
       content: "Hello!",
       width: 50
@@ -86,6 +88,7 @@ information.
 
     // A basic click-able image widget.
     widgets.Widget({
+      id: "google-link",
       label: "Widget with an image and a click handler",
       contentURL: "http://www.google.com/favicon.ico",
       onClick: function() {
@@ -95,6 +98,7 @@ information.
 
     // A widget that changes display on mouseover.
     widgets.Widget({
+      id: "mouseover-effect",
       label: "Widget with changing image on mouseover",
       contentURL: "http://www.yahoo.com/favicon.ico",
       onMouseover: function() {
@@ -107,6 +111,7 @@ information.
 
     // A widget that updates content on a timer.
     widgets.Widget({
+      id: "auto-update-widget",
       label: "Widget that updates content on a timer",
       content: "0",
       contentScript: 'setTimeout(function() {' +
@@ -117,6 +122,7 @@ information.
 
     // A widget that loads a random Flickr photo every 5 minutes.
     widgets.Widget({
+      id: "random-flickr",
       label: "Random Flickr Photo Widget",
       contentURL: "http://www.flickr.com/explore/",
       contentScriptWhen: "ready",
@@ -134,6 +140,7 @@ information.
 
     // A widget created with a specified width, that grows.
     let myWidget = widgets.Widget({
+      id: "widget-effect",
       label: "Wide widget that grows wider on a timer",
       content: "I'm getting longer.",
       width: 50,
@@ -144,6 +151,7 @@ information.
 
     // A widget communicating bi-directionally with a content script.
     let widget = widgets.Widget({
+      id: "message-test",
       label: "Bi-directional communication!",
       content: "<foo>bar</foo>",
       contentScriptWhen: "ready",
@@ -172,6 +180,11 @@ Represents a widget object.
     A required string description of the widget used for accessibility,
     title bars, and error reporting.
 
+  @prop id {string}
+    Mandatory string used to identify your widget in order to save it's 
+    location when user customizes it in the browser. 
+    This string has to be unique and must not be changed in time.
+
   @prop [content] {string}
     An optional string value containing the displayed content of the widget.
     It may contain HTML. Widgets must have either the `content` property or the

packages/addon-kit/lib/widget.js

@@ -107,7 +107,15 @@ const Widget = Trait.compose(Loader, Trait.compose({
     this._label = validate("label", options.label, valid.label);
 
     this.tooltip = "tooltip" in options ? options.tooltip : this._label
-
+
+    if ("id" in options)
+      this._id = options.id;
+    else
+      console.warn('You have to define an unique "id" attribute to your widget ' 
+        + 'in order to be able to remember its position.');
+
+    browserManager.validate(this._public);
+
     if ("width" in options)
       this.width = options.width;
     if ("panel" in options)
@@ -165,6 +173,9 @@ const Widget = Trait.compose(Loader, Trait.compose({
     }
   },
 
+  get id() this._id,
+  _id: null,
+
   get label() this._label,
   _label: null,
 
@@ -253,14 +264,25 @@ let browserManager = {
       }
     }
   },
-
-  // Registers an item with the manager. It's added to the add-on bar of
-  // all currently registered windows, and when new windows are registered it
-  // will be added to them, too.
-  addItem: function browserManager_addItem(item) {
+
+  // Used to validate widget by browserManager before adding it,
+  // in order to check input very early in widget constructor
+  validate : function (item) {
     let idx = this.items.indexOf(item);
     if (idx > -1)
       throw new Error("The widget " + item + " has already been added.");
+    if (item.id) {
+      let sameId = this.items.filter(function(i) i.id == item.id);
+      if (sameId.length > 0)
+        throw new Error("This widget ID is already used: " + item.id);
+    } else {
+      item.id = this.items.length;
+    }
+  },
+
+  // Registers an item with the manager. It's added to all currently registered 
+  // windows, and when new windows are registered it will be added to them, too.
+  addItem: function browserManager_addItem(item) {
     this.items.push(item);
     this.windows.forEach(function (w) w.addItems([item]));
   },
@@ -273,8 +295,8 @@ let browserManager = {
       this.windows.forEach(function (w) w.updateItem(item, property, value));
   },
 
-  // Unregisters an item from the manager.  It's removed from the addon-bar
-  // of all windows that are currently registered.
+  // Unregisters an item from the manager.  It's removed from all windows that 
+  // are currently registered.
   removeItem: function browserManager_removeItem(item) {
     let idx = this.items.indexOf(item);
     if (idx > -1) {
@@ -354,17 +376,6 @@ BrowserWindow.prototype = {
     return this._container;
   },
 
-  // Hide container
-  _hideContainer: function BW__hideContainer() {
-    if (this._container)
-      this._container.collapsed = true;
-  },
-
-  // Update the visibility state for the addon bar.
-  _onToggleUI: function BW__onToggleUI() {
-    this.container.collapsed = !this.container.collapsed;
-  },
-
   // Adds an array of items to the window.
   addItems: function BW_addItems(items) {
     items.forEach(this._addItemToWindow, this);
@@ -398,7 +409,15 @@ BrowserWindow.prototype = {
     // XUL element container for widget
     let node = this.doc.createElement("toolbaritem");
     let guid = require("xpcom").makeUuid().toString();
-    let id = "widget:" + guid;
+
+    // Temporary fix around require("self") failing on unit-test execution ...
+    let jetpackID = "testID";
+    try {
+      jetpackID = require("self").id;
+    } catch(e) {}
+
+    // Compute an unique and stable widget id with jetpack id and widget.id
+    let id = "widget:" + jetpackID + "-" + widget.id;
     node.setAttribute("id", id);
     node.setAttribute("label", widget.label);
     node.setAttribute("tooltiptext", widget.tooltip);
@@ -419,17 +438,51 @@ BrowserWindow.prototype = {
     let palette = toolbox.palette;
     palette.appendChild(node);
 
-    // Add the item to the toolbar
-    this.container.insertItem(id, null, null, false);
-
+    // Search for widget toolbar by reading toolbar's currentset attribute
+    let container = null;
+    let toolbars = this.doc.getElementsByTagName("toolbar");
+    for(let i = 0, l = toolbars.length; i < l; i++) {
+      let toolbar = toolbars[i];
+      if (toolbar.getAttribute("currentset").indexOf(id) == -1)
+        continue;
+      container = toolbar;
+    }
+
+    // if widget isn't in any toolbar, add it to the addon-bar
+    // TODO: we may want some "first-launch" module to do this only on very 
+    // first execution
+    if (!container) {
+      // TODO: find a way to make the following code work when we use "cfx run":
+      // http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#8586
+      // until then, force display of addon bar directly from sdk code
+      // https://bugzilla.mozilla.org/show_bug.cgi?id=627484
+      if (this.container.collapsed)
+        this.window.toggleAddonBar();
+      container = this.container;
+    }
+
+    // Now retrieve a reference to the next toolbar item
+    // by reading currentset attribute on the toolbar
+    let nextNode = null;
+    let currentSet = container.getAttribute("currentset");
+    let ids = (currentSet == "__empty") ? [] : currentSet.split(",");
+    let idx = ids.indexOf(id);
+    if (idx != -1) {
+      for(let i = idx; i < ids.length; i++) {
+        nextNode = this.doc.getElementById(ids[i]);
+        if (nextNode)
+          break;
+      }
+    }
+
+    // Finally insert our widget in the right toolbar and in the right position
+    container.insertItem(id, nextNode, null, false);
+
     let item = {widget: widget, node: node};
 
     this._fillItem(item);
 
     this._items.push(item);
-
-    if (this.container.collapsed)
-      this._onToggleUI();
   },
 
   // Initial population of a widget's content.
@@ -567,7 +620,7 @@ BrowserWindow.prototype = {
         for (let [type, listener] in Iterator(entry.eventListeners))
           entry.node.firstElementChild.removeEventListener(type, listener, true);
         // remove dom node
-        this.container.removeChild(entry.node);
+        entry.node.parentNode.removeChild(entry.node);
         // remove entry
         this._items.splice(this._items.indexOf(entry), 1);
         // cleanup symbiont
@@ -578,10 +631,6 @@ BrowserWindow.prototype = {
         entry.symbiont = null;
       }
     }, this);
-
-    // remove the add-on bar if no more items
-    if (this.container.getElementsByTagName("toolbaritem").length == 0)
-      this._hideContainer();
   },
 
   // Undoes all modifications to the window. The BrowserWindow