Widget persistent on toolbars and position #118
Conversation
By adding a unique and stable id to widgets and searching for the previous toolbar and position of the widget.
| let jetpackID = "testID"; | ||
| try{ | ||
| jetpackID = require("self").id; | ||
| }catch(e){} |
There was a problem hiding this comment.
nit: spaces after brackets, eg: s/try{/try {/ and s/}catch/} catch/
…der to let firefox code handle this. Fix unit test by notifying firefox code about addon install/uninstall.
|
I'm not really convinced by the id attribute addition. By writing the documentation it seems pretty clear that this attribute has not much sense for the API user ... May be we could "just" force developers to give unique labels ? Your thoughts ? |
maintaining location on widget create->destroy->create again, with same label
Instead suggest to provide widget.id by sending a warning message on id miss. And use widget instance number as id if there is no id provided by the developer.
| @prop id {string} | ||
| Optional string used to define a widget identifier. Mandatory if you want | ||
| to remember widget location in firefox. | ||
|
|
There was a problem hiding this comment.
I would be a bit more scary here. It's only optional for backwards compatibility. We should make the text convey that they must provide an id if they want to avoid constant bug reports and poor user experience.
You should add also that must be unique to the widget, cannot change ever, and maybe give an example.
Make it almost mandatory.
|
I didn't changed stuff about jetpackID. So using jetpackID is the best unique and stable ID for an addon as it will be the identifier used by AMO to update addon and so developers will end up having a stable jetpackID. |
| 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) |
There was a problem hiding this comment.
nit: spaces before/after operators
|
Landed as unique: |
This pull request is about these two bugs:
https://bugzilla.mozilla.org/show_bug.cgi?id=579505
https://bugzilla.mozilla.org/show_bug.cgi?id=617546
It adds a stable id to widgets by using widget.id attribute or a base64 convertion of widget.label.
Then this ID allow to search for the previous widget position:
1/ which toolbar, by reading toolbars currentset DOM attribute
2/ position in toolbar by parsing this same attribute
This fix errors on unload too, when we were trying to remove widget from addon-bar whereas they were moved to another toolbar.
There is one bug left around this piece of code, where I added a TODO comment:
Add-on bar toolbar gets displayed on every restart after being manually closed
https://bugzilla.mozilla.org/show_bug.cgi?id=627484
Dietrich, you are trying to solve this from Firefox side, but I think we can fix this from addon-sdk side, by opening the addon-bar only on first extension launch.