787390 - (sdk/ui/toolbar) Add Toolbar's components type

Status: RESOLVED FIXED

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

Description

[Matteo Ferretti [:zer0] [:matteo]]

Once the Add-on toolbar will be moved on the top, we need a way to display those widgets that are too long to be displayed in the Add-on bar.

As the UX mockups shows:

http://people.mozilla.com/~shorlander/files/addons-in-toolbar-i01/addons-in-toolbar.html

Those widgets will be represent as icon that toggle a toolbar with the full content.

Comment 1

[Matteo Ferretti [:zer0] [:matteo]]

When the work on this type of widget will start, during the brainstorming with UX check with the if Bug 645506 ("widgets should be auto-resizeable based on content") needs to be reopened. If it won't, then check also the Bug 737170 ("Widget should accept 'em' as a width unit").

Comment 2

[Erik Vold [:erikvold] (please needinfo? me)]

Why don't we just extend widgets to let them go anywhere, and then make it easy to create a toolbar which can host a widget or xul?

Comment 3

[Matteo Ferretti [:zer0] [:matteo]]

The bug was created log time ago, before we actually have the proper specs and get some decision; so I renamed it to avoid confusion.

Widget – the SDK Widget class – are going to be deprecated in the long term, together with the Add-on Bar.

Toolbar is going to be a sort of replacement for this add-ons that was based on "long size" widget and full HTML inside – like, for instance canvas.
This component will be, in fact, the only one to allow HTML content – Sidebar is out of scope at the moment, we also have special case for it that we have to deal with.

We already come up with some ideas for this component API, that I'd love to discuss with you; let's schedule a meeting this week!

Comment 4

[Erik Vold [:erikvold] (please needinfo? me)]

Dave and Matteo can you take a look at https://github.com/mozilla/addon-sdk/wiki/JEP-Toolbars ?

I've made my plan clear there, it's very simple.  Let me know what you think!

Comment 5

[Dave Townsend [:mossop]]

Awesome, thanks Erik. Can you add a link to an etherpad page and we can add our thoughts in there.

Comment 6

[Erik Vold [:erikvold] (please needinfo? me)]

Added this link: 

https://etherpad.mozilla.org/jetpack-toolbar

Comment 7

[Erik Vold [:erikvold] (please needinfo? me)]

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/988

Pointer to Github pull-request

Comment 8

[Erik Vold [:erikvold] (please needinfo? me)]

Comment on attachment 745238 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/988

I've got a little prototype ready for some eyes, this should make what I have planned for bug 838797 more clear too.

Comment 9

[Matteo Ferretti [:zer0] [:matteo]]

Yeah, that's exactly what I tried to explain in the bug 838797 that I'd like to avoid. What we need is a way to handle "views" and "models" and keep them in sync across windows; decoupling that from the actual "view" implementation: it shouldn't be important if they're XUL or HTML or whatever. Here it seems it's too "xul centric" where I really want to keep distance from it, and having a more flexible pattern, hiding the concrete implementation of the view.

The code Irakli wrote for the new Panel is already a good point where to start.

This pattern also should be able to handle in a good way the problems exposed in the etherpad about Multiple and Shared State, with Multiple Views. This is what I'm currently focus on, and I'm trying to solve in an elegant way but it's tricky – a good approach could be more functional but the API will be too much different from what we have no, so I'm still thinking about it.

As prototype is really good, but for what I've in mind for the new UI components I'd like to decoupling more from XUL implementation.

Comment 10

[Matteo Ferretti [:zer0] [:matteo]]

In addition, I see `Toolbar` as a "list of components", where add-on devs can add high level UI component like Button and 'WebView', so you don't have actually to create a lot of xul nodes, but just the ones related to the toolbar itself: all the buttons and the 'WebView' have their own nodes create as "view".

About the "list of components" thing, we could make `Toolbar` implements `List`, so it will be consistent with the rest of our API, and add-on devs will be able to do also things like:

    for (let component of toolbar)
        console.log(component.id);

 etc.

Comment 11

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Comment on attachment 745238 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/988

I wanna be clear in regards the fact that I don't really care much for the implementation details as long as it does not add unnecessary complexity. I would also defer decisions on implementation details to ZER0 as an owner of this goal.

That being said I would do it in opposite way similar to panel. Where toolbar construct is just defined as a data structure and stored and on `browser/events#DOMContentLoaded` view for the given state is created.

As of events I would follow the pattern established lately, like browser/events, tab/events etc.. where events from all browsers are mixed into toolbar/events or such.

Comment 12

[Dave Townsend [:mossop]]

Comment on attachment 745238 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/988

I like the XUL module here, though it isn't actually XUL specific, just DOM so maybe it should be called that instead? Comments about managing state across windows that I made in the sidebar bug apply here too.

Comment 13

[Erik Vold [:erikvold] (please needinfo? me)]

Not working on this anymore.

Comment 14

[Erik Vold [:erikvold] (please needinfo? me)]

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1270

Pointer to Github pull-request

Comment 15

[Erik Vold [:erikvold] (please needinfo? me)]

Comment on attachment 819591 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1270

Hey Dave,

Can you take a look at this? there is a work around implemented here for remembering a toolbar's collapsed state between sessions/restarts using `document.persist` which I think sucks, and I can't think of a better hack.  Also we don't have a way to test this atm, and won't unless we update cfx.

Comment 16

[Erik Vold [:erikvold] (please needinfo? me)]

Hey Matteo,  pinging you on this, it's been waiting for a review for a while now.

You asked in one of our meetings, I think 2 weeks ago, why the toolbar takes a `url` property instead of allowing items to be added to it like and `HTMLView` that was discussed in Q2 so I'll write down my reasoning here in case you were waiting for that, which I discussed with Dave in our 1-1 at the start of Q3.

Originally when I faced this bug and started the JEP in Q2 I suggested an `appendChild` method which would allow devs to add a `Widget` or `Button` or other things.  After awhile I heard that it was decided that Widget would be deprecated, then after Q2 was over I realized how the button & sidebar modules were implemented and that xul elements were being ignored that it would be very difficult to add these buttons to toolbars (on the implementation side), or using an HTMLView with Sidebars (this would require writing a completely fresh sidebar implementation which ignores the m-c one or require a large monkey patch on the m-c side), so the task started looking far bigger than I originally thought it would be.  Also HTMLView was not JEP'd it's very different from our other APIs (ie we need to create a HTMLView instance and attach it to viewable areas, instead of merely creating a UI component).

So to repeat, these are reasons:

1. It's new, so if current APIs (Panel, Sidebar, Tabs) were to support HTMLView in some way, then they would have to be updated in some way which Toolbars could do as well.

2. It's complex, just writing/reading the JEP would be daunting (and ugly) imo; an HTMLView that can move around UI components is an abstraction that I think web devs would find unpleasing (even if it is more powerful).  That said we still don't know if we are targeting web devs or not..

3. Maintainability costs, I think the amount of code required to support this feature would be not only massive but also much harder to understand.  Since m-c sidebars do not support sidebars with browser elements that stay alive during open/close we would have to either patch m-c so that it is easy to do so, or monkey patch Fx with sdk code to do the same.  Also if/when we support Sidebars on Fennec (just for an example) supporting a HTMLView api over our current pattern I fear would be much harder to do.

Anyhow I'm not totally against HTMLView and if you think it is the correct way to go then I still think we can support it in the future (using the url option would be the left most HTMLVIew and more could be added via a `toolbar.appendChild(..)` method), or we can put this on hold until it's been JEP'd.

Irakli we should get your feedback on this too.

I should've JEP'd this and gotten agreement sooner so I apologize for that, I put it on hold until we JEP'd the state management stuff, and then Dave said he wanted me to post what I had done and get it reviewed, so I did that sooner than I otherwise would have for his sake.

There is a problem with showing toolbars after the browser restarts which I mentioned in comment 15 which prevents us from landing this until we have a solution anyhow.  Also once this issue is resolved we need a way to test it which our current test frame work does not provide.  So I do not expect a r+ here I mainly wanted feedback and to fulfill Dave's request that I post this and get it reviewed asap.

Comment 17

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #15)
> Comment on attachment 819591 [details]
> Pointer to Github pull request:
> https://github.com/mozilla/addon-sdk/pull/1270
> 
> Hey Dave,
> 
> Can you take a look at this? there is a work around implemented here for
> remembering a toolbar's collapsed state between sessions/restarts using
> `document.persist` which I think sucks, and I can't think of a better hack. 
> Also we don't have a way to test this atm, and won't unless we update cfx.

Dave I asked you a question here which I don't think you noticed.

Comment 18

[Dave Townsend [:mossop]]

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #17)
> (In reply to Erik Vold [:erikvold] [:ztatic] from comment #15)
> > Comment on attachment 819591 [details]
> > Pointer to Github pull request:
> > https://github.com/mozilla/addon-sdk/pull/1270
> > 
> > Hey Dave,
> > 
> > Can you take a look at this? there is a work around implemented here for
> > remembering a toolbar's collapsed state between sessions/restarts using
> > `document.persist` which I think sucks, and I can't think of a better hack. 
> > Also we don't have a way to test this atm, and won't unless we update cfx.
> 
> Dave I asked you a question here which I don't think you noticed.

I'm not sure what question you're referring to here. For the collapsed state, if it works then I think that's as reasonable place as any to store the collapsed state, that is what localstore is for, the only alternative that springs to mind is using preferences.

As for testing, can't we handle that with two test add-ons, one runs, creates a toolbar and collapses it, the second runs, declares the same toolbar and verifies that it is collapsed? Even opening a new windows seems like it would be decent coverage here.

Comment 19

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Dave Townsend (:Mossop) from comment #18)
> (In reply to Erik Vold [:erikvold] [:ztatic] from comment #17)
> > (In reply to Erik Vold [:erikvold] [:ztatic] from comment #15)
> > > Comment on attachment 819591 [details]
> > > Pointer to Github pull request:
> > > https://github.com/mozilla/addon-sdk/pull/1270
> > > 
> > > Hey Dave,
> > > 
> > > Can you take a look at this? there is a work around implemented here for
> > > remembering a toolbar's collapsed state between sessions/restarts using
> > > `document.persist` which I think sucks, and I can't think of a better hack. 
> > > Also we don't have a way to test this atm, and won't unless we update cfx.
> > 
> > Dave I asked you a question here which I don't think you noticed.
> 
> I'm not sure what question you're referring to here. For the collapsed
> state, if it works then I think that's as reasonable place as any to store
> the collapsed state, that is what localstore is for, the only alternative
> that springs to mind is using preferences.


Ah you got it now, thanks.  I'll try to weigh the options.


> As for testing, can't we handle that with two test add-ons, one runs,
> creates a toolbar and collapses it, the second runs, declares the same
> toolbar and verifies that it is collapsed? Even opening a new windows seems
> like it would be decent coverage here.

Hmm clever, that might work!

Comment 20

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #16)
> Hey Matteo,  pinging you on this, it's been waiting for a review for a while
> now.
> 
> You asked in one of our meetings, I think 2 weeks ago, why the toolbar takes
> a `url` property instead of allowing items to be added to it like and
> `HTMLView` that was discussed in Q2 so I'll write down my reasoning here in
> case you were waiting for that, which I discussed with Dave in our 1-1 at
> the start of Q3.
> 
> Originally when I faced this bug and started the JEP in Q2 I suggested an
> `appendChild` method which would allow devs to add a `Widget` or `Button` or
> other things.  

I believe that's what we have in JEP.

> After awhile I heard that it was decided that Widget would be
> deprecated, then after Q2 was over I realized how the button & sidebar
> modules were implemented and that xul elements were being ignored that it
> would be very difficult to add these buttons to toolbars (on the
> implementation side), 

I'm not quite sure I understand the issue, or why the implementation is so difficult.
Maybe having some more context and discussion will let us discover elegant solution.

I also in general don't think we should make API decisions based on implementation
difficulty. The point of SDK is that we do a difficult work so that our users don't
have to. That being said, there may be cases where it's better to avoid difficulties.
I can't really make any assessments given the limited context.

Maybe it's worth having email thread on mailing list with me & Matteo in order to
discuss implementation details.


> or using an HTMLView with Sidebars (this would require
> writing a completely fresh sidebar implementation which ignores the m-c one
> or require a large monkey patch on the m-c side), so the task started
> looking far bigger than I originally thought it would be.  

I don't understand what's the relation with a side bar.

> Also HTMLView was
> not JEP'd it's very different from our other APIs (ie we need to create a
> HTMLView instance and attach it to viewable areas, instead of merely
> creating a UI component).
> 

I think there are many assumptions about HTMLView, how is it supposed to be used
or implemented. As far as I recall (from a brief conversation) HTMLView meant to be
a widget replacement with a reduced features to avoid problems we had / have with
widget API.
 

>
> So to repeat, these are reasons:
> 
> 1. It's new, so if current APIs (Panel, Sidebar, Tabs) were to support
> HTMLView in some way, then they would have to be updated in some way which
> Toolbars could do as well.
>

I still don't understand how HTMLView requires changes to any of the existing APIs
and why all of that bounded to Toolbar API.

> 
> 2. It's complex, just writing/reading the JEP would be daunting (and ugly)
> imo; an HTMLView that can move around UI components is an abstraction that I
> think web devs would find unpleasing (even if it is more powerful).  That
> said we still don't know if we are targeting web devs or not..
>

I feel like your have some specific idea about what HTMLView should be which I
don't think matches at least mine. In my mind if we do an HTMLView it basically
will be an equivalent of an iframe to which messages can be posted or received
from. Also document loaded in it will have to be bundled with add-on.


> 
> 3. Maintainability costs, I think the amount of code required to support
> this feature would be not only massive but also much harder to understand. 

What if we renamed Toolbar to HTMLView ? Not that I suggest that, but I don't
understand why HTMLView is more complicated that Toolbar or Widget. Maybe we
don't can reduce that API to a degree where it's no longer complicated ?

> Since m-c sidebars do not support sidebars with browser elements that stay
> alive during open/close we would have to either patch m-c so that it is easy
> to do so, or monkey patch Fx with sdk code to do the same. 

I don't see the connection with sidebar or with persistence. 

> Also if/when we
> support Sidebars on Fennec (just for an example) supporting a HTMLView api
> over our current pattern I fear would be much harder to do.
>

Now I'm completely lost.


> 
> Anyhow I'm not totally against HTMLView and if you think it is the correct
> way to go then I still think we can support it in the future (using the url
> option would be the left most HTMLVIew and more could be added via a
> `toolbar.appendChild(..)` method), or we can put this on hold until it's
> been JEP'd.
>

I don't think that toolbar.appendChild(x) along with toolbar.url = "./index.html" is
a good API. As far as I can see toolbar JEP is all about creating toolbar where
you can add / remove other components to. There is no mention of HTMLView or url
so why are we even talking about it here.

I do understand that we'll want to let users to build more flexible UI than just
buttons, but I think those things can be added as definition of `x` in
`toolbar.appendChild(x)`.

HTMLView or even better iframe can be one of the `x`'s and I'm sure we can find a
good balance in terms of implementation complexity and flexibility.

I don't really see why Toolbar has to be put on hold. It can just implement
agreed API (one in the JEP), more `x` types can be added in a separate patches.
  

>
> Irakli we should get your feedback on this too.
> 
> I should've JEP'd this and gotten agreement sooner so I apologize for that,
> I put it on hold until we JEP'd the state management stuff, and then Dave
> said he wanted me to post what I had done and get it reviewed, so I did that
> sooner than I otherwise would have for his sake.
>

Wait you did wrote a JEP and we did merged it inn:
https://github.com/mozilla/addon-sdk/blob/JEP/Toolbars.md

Or do you refer to an HTMLView ?

> 
> There is a problem with showing toolbars after the browser restarts which I
> mentioned in comment 15 which prevents us from landing this until we have a
> solution anyhow.  Also once this issue is resolved we need a way to test it
> which our current test frame work does not provide.  So I do not expect a r+
> here I mainly wanted feedback and to fulfill Dave's request that I post this
> and get it reviewed asap.

Comment 21

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

So I have skimmed through the pull request and as far as I can see this implementation
has a quite different API than one in JEP. I think I understand better the comments
above now, although I'm not convinced in arguments given, mainly because you make several
assumptions and there are other alternatives (which you may have not considered yet).

Here are few reasons I remember for designing toolbar API as described by JEP:

1. Toolbar is just a container for other UI components, which is great because
   we can implement toolbars on all platforms, but maybe not all the components.
   In other words on fennec we may have toolbars with buttons and icons, but not
   with HTMLView in them.

2. We can optimize simple add-on use cases a lot more, both in terms of API and performance.
   If add-on just needs to make bunch of buttons, there's no need for them to do message
   passing and state handling.

3. In a future more simple components can be implemented that will be still more limited
   than HTML but still could be supported on all platforms. Maybe text field, label or
   an Image.

I think all above reasons are still valid. As of HTML view I think we're just talking of
simplified widget replacement, so if anything it will not be more complex than widget.

If you think having a JEP for HTMLView or prior discussions would help to make sure Toolbar
is on right path, let's do it. Although I personally would not block Toolbar API on this.

Also to be honest I'd adjust API a little more to not only allow adding items but also
removing them. In addition giving list of items at the construction time IMO would be a good
idea.

Comment 22

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Irakli Gozalishvili [:irakli] [:gozala] [@gozala] from comment #20)
> (In reply to Erik Vold [:erikvold] [:ztatic] from comment #16)
> >
> > Irakli we should get your feedback on this too.
> > 
> > I should've JEP'd this and gotten agreement sooner so I apologize for that,
> > I put it on hold until we JEP'd the state management stuff, and then Dave
> > said he wanted me to post what I had done and get it reviewed, so I did that
> > sooner than I otherwise would have for his sake.
> >
> 
> Wait you did wrote a JEP and we did merged it inn:
> https://github.com/mozilla/addon-sdk/blob/JEP/Toolbars.md
>

Sigh,  I wrote the JEP, but even I never r+'d it, and nobody else did either as far as I recall.  So why it is you pulled it in to the JEP branch is beyond me.

I wrote this JEP right after writing the XUL JEP which you and Matteo r-'d, and in my mind this JEP depended on that one for the implementation reasons that I refer to in comment 16.
 
> Or do you refer to an HTMLView ?

I have no idea what you are asking here.

Comment 23

[Erik Vold [:erikvold] (please needinfo? me)]

Irakli, I have to say he fact that you think the JEP here was approved and agreed on concerns me, why do you think that?

Comment 24

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Irakli Gozalishvili [:irakli] [:gozala] [@gozala] from comment #20)
> (In reply to Erik Vold [:erikvold] [:ztatic] from comment #16)
> > 
> > 2. It's complex, just writing/reading the JEP would be daunting (and ugly)
> > imo; an HTMLView that can move around UI components is an abstraction that I
> > think web devs would find unpleasing (even if it is more powerful).  That
> > said we still don't know if we are targeting web devs or not..
> >
> 
> I feel like your have some specific idea about what HTMLView should be which
> I
> don't think matches at least mine. In my mind if we do an HTMLView it
> basically
> will be an equivalent of an iframe to which messages can be posted or
> received
> from. Also document loaded in it will have to be bundled with add-on.

I think I understand your idea, it sounds like it's a rewrite of widget and rename called HTMLView.

Widgets were only displayable in toolbars before, and so will HTMLView afaict.

If and how you wish to use HTMLView in other modules eludes me, and if you do not plan this then why we should create this new name for Widgets eludes me.  And I was told to not use Widgets because they are being deprecated, which leads to me this question: Why are we deprecating Widgets and replacing it with something that is pretty much the same?  in other words what are the advantages to the new thing?

Comment 25

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Irakli Gozalishvili [:irakli] [:gozala] [@gozala] from comment #20)
> (In reply to Erik Vold [:erikvold] [:ztatic] from comment #16)
> > Hey Matteo,  pinging you on this, it's been waiting for a review for a while
> > now.
> > 
> > You asked in one of our meetings, I think 2 weeks ago, why the toolbar takes
> > a `url` property instead of allowing items to be added to it like and
> > `HTMLView` that was discussed in Q2 so I'll write down my reasoning here in
> > case you were waiting for that, which I discussed with Dave in our 1-1 at
> > the start of Q3.
> > 
> > Originally when I faced this bug and started the JEP in Q2 I suggested an
> > `appendChild` method which would allow devs to add a `Widget` or `Button` or
> > other things.  
> 
> I believe that's what we have in JEP.

That JEP wasn't finalized nor agreed to, the fact that Widget is used everywhere in it should be a hint.

> > After awhile I heard that it was decided that Widget would be
> > deprecated, then after Q2 was over I realized how the button & sidebar
> > modules were implemented and that xul elements were being ignored that it
> > would be very difficult to add these buttons to toolbars (on the
> > implementation side), 
> 
> I'm not quite sure I understand the issue, or why the implementation is so
> difficult.
> Maybe having some more context and discussion will let us discover elegant
> solution.
> 
> I also in general don't think we should make API decisions based on
> implementation
> difficulty.

In general I agree, obviously that rule must have it's limits tho, and we probably disagree on where that limit should be, based on our previous discussions on how the panel module was implemented.

> The point of SDK is that we do a difficult work so that our
> users don't
> have to.

That is right, but doing unnecessary hard work is not one of our goals afaik.  Anyhow we have no manifesto still, which we all agreed was needed iirc, and Mossop said he would work on, so the reasons why Jetpack exist are still up for debate based on our conversation at the last work week.

> That being said, there may be cases where it's better to avoid
> difficulties.
> I can't really make any assessments given the limited context.
> 
> Maybe it's worth having email thread on mailing list with me & Matteo in
> order to
> discuss implementation details.
> 

Well given that it looks like you want a complete rewrite here I will have to put it on hold until after the AOM project, because this is starting to look like a time suck.

Perhaps you should JEP HTMLView in the meantime Irakli, I am particularly interested in how you see HTMLView being used..

Comment 26

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #22)
> (In reply to Irakli Gozalishvili [:irakli] [:gozala] [@gozala] from comment
> #20)
> > (In reply to Erik Vold [:erikvold] [:ztatic] from comment #16)
> > >
> > > Irakli we should get your feedback on this too.
> > > 
> > > I should've JEP'd this and gotten agreement sooner so I apologize for that,
> > > I put it on hold until we JEP'd the state management stuff, and then Dave
> > > said he wanted me to post what I had done and get it reviewed, so I did that
> > > sooner than I otherwise would have for his sake.
> > >
> > 
> > Wait you did wrote a JEP and we did merged it inn:
> > https://github.com/mozilla/addon-sdk/blob/JEP/Toolbars.md
> >
> 
> Sigh,  I wrote the JEP, but even I never r+'d it, and nobody else did either
> as far as I recall.  So why it is you pulled it in to the JEP branch is
> beyond me.
> 
> I wrote this JEP right after writing the XUL JEP which you and Matteo r-'d,
> and in my mind this JEP depended on that one for the implementation reasons
> that I refer to in comment 16.
>  
> > Or do you refer to an HTMLView ?
> 
> I have no idea what you are asking here.

I just moved all JEP wiki pages to JEP branch after we decided to use github pull requests.

Comment 27

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

 (In reply to Erik Vold [:erikvold] [:ztatic] from comment #23)
> Irakli, I have to say he fact that you think the JEP here was approved and
> agreed on concerns me, why do you think that?

Because JEP was written along with other new UX JEPs which we gathered feedback on
etherpad, which from what I can see was very positive. Also I believe we talked
about this during last work week and we were all in agreement.

It is true that since then we moved to pull request based JEP reviews, but as I said
earlier all the JEPs that were on wiki and had being through a feedback loop were
merged in by me.

If you think that JEP was not agreed upon, I can remove it and we can do another
proposal loop.

Comment 28

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #25)
> (In reply to Irakli Gozalishvili [:irakli] [:gozala] [@gozala] from comment
> #20)
> > (In reply to Erik Vold [:erikvold] [:ztatic] from comment #16)
> > > Hey Matteo,  pinging you on this, it's been waiting for a review for a while
> > > now.
> > > 
> > > You asked in one of our meetings, I think 2 weeks ago, why the toolbar takes
> > > a `url` property instead of allowing items to be added to it like and
> > > `HTMLView` that was discussed in Q2 so I'll write down my reasoning here in
> > > case you were waiting for that, which I discussed with Dave in our 1-1 at
> > > the start of Q3.
> > > 
> > > Originally when I faced this bug and started the JEP in Q2 I suggested an
> > > `appendChild` method which would allow devs to add a `Widget` or `Button` or
> > > other things.  
> > 
> > I believe that's what we have in JEP.
> 
> That JEP wasn't finalized nor agreed to, the fact that Widget is used
> everywhere in it should be a hint.
> 
> > > After awhile I heard that it was decided that Widget would be
> > > deprecated, then after Q2 was over I realized how the button & sidebar
> > > modules were implemented and that xul elements were being ignored that it
> > > would be very difficult to add these buttons to toolbars (on the
> > > implementation side), 
> > 
> > I'm not quite sure I understand the issue, or why the implementation is so
> > difficult.
> > Maybe having some more context and discussion will let us discover elegant
> > solution.
> > 
> > I also in general don't think we should make API decisions based on
> > implementation
> > difficulty.
> 
> In general I agree, obviously that rule must have it's limits tho, and we
> probably disagree on where that limit should be, based on our previous
> discussions on how the panel module was implemented.
> 
> > The point of SDK is that we do a difficult work so that our
> > users don't
> > have to.
> 
> That is right, but doing unnecessary hard work is not one of our goals
> afaik.  Anyhow we have no manifesto still, which we all agreed was needed
> iirc, and Mossop said he would work on, so the reasons why Jetpack exist are
> still up for debate based on our conversation at the last work week.
> 
> > That being said, there may be cases where it's better to avoid
> > difficulties.
> > I can't really make any assessments given the limited context.
> > 
> > Maybe it's worth having email thread on mailing list with me & Matteo in
> > order to
> > discuss implementation details.
> > 
> 
> Well given that it looks like you want a complete rewrite here I will have
> to put it on hold until after the AOM project, because this is starting to
> look like a time suck.
> 
> Perhaps you should JEP HTMLView in the meantime Irakli, I am particularly
> interested in how you see HTMLView being used..

Yes I think HTMLView (terrible name BTW) should mainly be a replacement to a current
widget API (at least in the first cut). Main reason is that widget does supports
remote content which we no longer wish to do for security reasons. This also means
that content scripts for widgets are obsolete since you can directly communicate with
trusted content through message passing. In other words HTMLView is a Widget with a
simpler API and better security. The reason we don't simply modify widget API is to
let users migrate without breaking their add-ons.

Comment 29

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #25)
> (In reply to Irakli Gozalishvili [:irakli] [:gozala] [@gozala] from comment
> #20)
> > (In reply to Erik Vold [:erikvold] [:ztatic] from comment #16)
> > > Hey Matteo,  pinging you on this, it's been waiting for a review for a while
> > > now.
> > > 
> > > You asked in one of our meetings, I think 2 weeks ago, why the toolbar takes
> > > a `url` property instead of allowing items to be added to it like and
> > > `HTMLView` that was discussed in Q2 so I'll write down my reasoning here in
> > > case you were waiting for that, which I discussed with Dave in our 1-1 at
> > > the start of Q3.
> > > 
> > > Originally when I faced this bug and started the JEP in Q2 I suggested an
> > > `appendChild` method which would allow devs to add a `Widget` or `Button` or
> > > other things.  
> > 
> > I believe that's what we have in JEP.
> 
> That JEP wasn't finalized nor agreed to, the fact that Widget is used
> everywhere in it should be a hint.
> 
> > > After awhile I heard that it was decided that Widget would be
> > > deprecated, then after Q2 was over I realized how the button & sidebar
> > > modules were implemented and that xul elements were being ignored that it
> > > would be very difficult to add these buttons to toolbars (on the
> > > implementation side), 
> > 
> > I'm not quite sure I understand the issue, or why the implementation is so
> > difficult.
> > Maybe having some more context and discussion will let us discover elegant
> > solution.
> > 
> > I also in general don't think we should make API decisions based on
> > implementation
> > difficulty.
> 
> In general I agree, obviously that rule must have it's limits tho, and we
> probably disagree on where that limit should be, based on our previous
> discussions on how the panel module was implemented.
> 
> > The point of SDK is that we do a difficult work so that our
> > users don't
> > have to.
> 
> That is right, but doing unnecessary hard work is not one of our goals
> afaik.  Anyhow we have no manifesto still, which we all agreed was needed
> iirc, and Mossop said he would work on, so the reasons why Jetpack exist are
> still up for debate based on our conversation at the last work week.
> 
> > That being said, there may be cases where it's better to avoid
> > difficulties.
> > I can't really make any assessments given the limited context.
> > 
> > Maybe it's worth having email thread on mailing list with me & Matteo in
> > order to
> > discuss implementation details.
> > 
> 
> Well given that it looks like you want a complete rewrite here I will have
> to put it on hold until after the AOM project, because this is starting to
> look like a time suck.
>

I don't want to rewrite anything. Clearly JEP states one API which had positive
feedback and as far as I understand ZER0 was expecting implementation of that API
too. So to me only one who does not agrees with proposed API is you, which is ironic
since you wrote it.

Now that aside, as already pointed out, I don't think toolbar API should be blocked
by HTMLView, neither it should care about components that it will contain. Only thing
toolbar should do is contain UI components from which it can get actual views (xul nodes)
and append / remove from itself.

> 
> Perhaps you should JEP HTMLView in the meantime Irakli, I am particularly
> interested in how you see HTMLView being used..

Matteo agreed to take over writing HTMLView JEP, so I assigned him Bug 942158.

Comment 30

[Matteo Ferretti [:zer0] [:matteo]]

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #16)

> I should've JEP'd this and gotten agreement sooner so I apologize for that,
> I put it on hold until we JEP'd the state management stuff, and then Dave
> said he wanted me to post what I had done and get it reviewed, so I did that
> sooner than I otherwise would have for his sake.
> 
> There is a problem with showing toolbars after the browser restarts which I
> mentioned in comment 15 which prevents us from landing this until we have a
> solution anyhow.  Also once this issue is resolved we need a way to test it
> which our current test frame work does not provide.  So I do not expect a r+
> here I mainly wanted feedback and to fulfill Dave's request that I post this
> and get it reviewed asap.

Hi Erik, thanks a lot for your explanation. I took a look to the pull, and also exchange some thoughts with Irakli.
I would set r- for the reason exposed there, and because is not consistent with any JEP we have at the moment. I still agree with what Irakli said in comment 21, and I understand it would take more time to implement, but basically Toolbar needs to be an high level API because is just a container for, at least, the new Button thing – HTMLView could be implemented later.
Just having toolbar without any way to add anything rather than access directly to DOM, it doesn't serve our purposes; it needs at least a way to add our new buttons.

Comment 31

[Erik Vold [:erikvold] (please needinfo? me)]

To be clear everyone, I was fine with the JEP that I wrote, I was not fine with making the Toolbar module and updating the Buttons module to work with Toolbars (after I've seen the way the Button module was implemented) and I was not fine with rewriting Widgets all as part of this project.  Furthermore that desire was never made clear to me.  When I was given this project I was just told to do the Toolbar module.
At that point there was a dicussion on vidyo with Dave and Matteo present I think Jeff and Irakli may have been there too where everyone was arguing for the current API that I have now implemented with a url.  The argument for this API was that devs could use html <button> so why would they want to add xul buttons to the toolbar?  and everything else will be harder in xul than html so what is the point?

Anyhow I find it change that everyone is so for that API I wrote on a whim only in connection with the XUL module suggestion which everyone was so against.  If you want that API than pull request 988 should be fine yes?

Anyhow the JEP that I wrote has Widget all over it, I never like the HTMLView idea, maybe I don't see the picture well enough, but what I see looks ugly.  This is why HTMLView never made it to the JEP, because I don't like the idea.  I would prefer to use Widgets.

It sounds like Widgets are being deprecated because they are too hard to implement/maintain, and Irakli has addressed that concern in comment "I also in general don't think we should make API decisions based on implementation difficulty. The point of SDK is that we do a difficult work so that our users don't have to."

So it sounds to me like Irkali and Matteo have decided (without informing me) that supporting Widgets is too hard and I think that deprecating them and replacing them with HTMLView is not only more difficult to implement and maintain but a less good overall picture of the future.

Also I think that implementing HTMLView is outside the scope of this bug, and should be done in another bug, which this one can depend on.  If that is the direction that we want to go then I shall wait for that implementation.

Comment 32

[Erik Vold [:erikvold] (please needinfo? me)]

Dave can you please read this thread when you have a chance and provide your 2c?  I'm sorry it is long, it's not pressingly urgent tho.

Comment 33

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1307

Pointer to Github pull-request

Comment 34

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

At this point I decided it was more effective to provide prototype implementation demonstrating
what I think JEP reflects and what implementation should be. It shows how HTMLView, Button or anything
else can provide support for toolbar.

ZER0 also had some arguments why Toolbar should not dynamically be added / removed items. Instead of
trying to post my interpretation I'd prefer Matteo to post it himself.

I hope this makes clear that there is no reason to block Toolbar implementation and that support for
items that toolbar can contain can be added in future changes.

Comment 35

[Dave Townsend [:mossop]]

We're about to enter our 9th straight month of work on UX APIs. That's too long. Some of that was caused by Australis churn. Too much has been caused by API and implementation churn (something which is as much my responsibility for not clamping down on as anyone else's). I dislike time-bounding implementation work but I have to draw a line somewhere. We need to land something here by the end of the year so we can move on to other things.

Irakli, I think your example is clear but unless HTMLView was implemented and Button and ToggleButton updated with the attachTo code it doesn't actually give us anything functional. Erik's implementation works and gives developers a lot of flexibility to build any kind of toolbar they want. I'm fully aware that that could come back and bite us but I don't see any other option getting implemented and landed in the next 3 weeks.

We should solve any obvious problems with the API Erik has created  (I didn't spot any mentioned so far other than it not matching the JEP) and review and land the implementation. We should probably call it a low-level/experimental API, maybe call it something like BaseToolbar or HTMLToolbar since I see this as something we can build on later if we think it is worth it. How developers make use of it and the feedback they give us will tell us whether we need to spend more time later implementing a higher level toolbar API that can accept multiple components like Button and ToggleButton.

Comment 36

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

(In reply to Dave Townsend (:Mossop) from comment #35)
> We're about to enter our 9th straight month of work on UX APIs. That's too
> long. Some of that was caused by Australis churn. Too much has been caused
> by API and implementation churn (something which is as much my
> responsibility for not clamping down on as anyone else's). I dislike
> time-bounding implementation work but I have to draw a line somewhere. We
> need to land something here by the end of the year so we can move on to
> other things.
> 
> Irakli, I think your example is clear but unless HTMLView was implemented
> and Button and ToggleButton updated with the attachTo code it doesn't
> actually give us anything functional. 

Only reason I have not implemented `attachTo` to an actual `Button` and `ToogleButton` is
because Matteo told he was doing some changes to them and I would only cause conflicts. Also
note that only change required for `Button` and `ToogleButton` is that they need to inherit
from general `Component` defined here (so we're talking about few lines change):
https://github.com/mozilla/addon-sdk/pull/1307/files#diff-de46563daced2d11639b8d1adbd12d25R16

 
Erik's implementation works and gives
> developers a lot of flexibility to build any kind of toolbar they want. 

I don't think this is true, that patch is not complete and does not place nicely with
CustomizableUI API provided by Australis.

> I'm
> fully aware that that could come back and bite us but I don't see any other
> option getting implemented and landed in the next 3 weeks.
>

While I do understand desire of having toolbar API in 3 weeks, I don't agree that
going with incomplete implementation that doesn't matches either proposed & earlier
discussed API is a reasonable option. I also think that 3 weeks is enough to finish
either implementation.

> 
> We should solve any obvious problems with the API Erik has created  (I
> didn't spot any mentioned so far other than it not matching the JEP) and
> review and land the implementation. 

Well, I believe we discussed toolbars to large extend and JEP reflects what we agreed
made most sense and avoided all the issues we've in the past. I have not went through
specifics because some of them have being discussed verbally and others are in the
etherpad (which also includes your comments BTW). I feel like we're back to square one
now, so I'll try to summarize why I don't like this API and trying to defend what we
agreed upon earlier:

1. This API implies that any kind of toolbar needs full blown HTML document presumably in
   iframe. This also implies that that document would need to communicate with add-on somehow
   presumable through message passing. In nutshell these are same issues what made pushed
   us towards deprecating widgets.


   If you look at how devs use the widgets it's easy to put them in following groups:
 
   A) Buttons
   B) Visuals (graps, weather, etc...)
   C) Everything else
   
   Most of the use cases could have being covered by simple APIs like new Buttons & maybe remote
   canvas. Making message passing a tool for only C use case (which btw isn't common).
   I don't even mention how much costly it is to have iframe with document just to render a button
   or icon, main issues is dev experience.

   Bigger space of toolbars doesn't actually makes it better, quite the contrary. Devs would need
   to build their buttons in HTML and set listeners then send messages to add-on host and loop
   back, which is most common complain we get.

   Now if we implement JEP proposal devs could just create buttons and handle clicks right from the
   add-on code. Not to mention that many tab / window / specific states will be also taken cared
   for them. So `HTMLView` will only remain for drawing something advanced. I also hope that we would
   learn from our users and provide more things like canvas that can be added to panel that would
   cover big chunk of use cases and simplify task for our users.

2. This toolbar API is not extendable, so we will have to either do what we're doing with widget API
   or think harder before we make the same mistake. Not to mention that it is awkward that other users
   will be able to drop buttons to other toolbars but not to SDK ones. Or that devs could create
   simple buttons using our APIs but not use them with our toolbars.

3. This toolbar API is not very portable across platforms, given limitation of platform in some cases
   & hardware it runs in other cases. On the other hand support for toolbars with just buttons would
   be quite visible. I'm not implying to ditch advanced cases that would require full blown document,
   but those cases could be luxury on the more powerful platforms like desktop.


> We should probably call it a
> low-level/experimental API, maybe call it something like BaseToolbar or
> HTMLToolbar since I see this as something we can build on later if we think
> it is worth it. How developers make use of it and the feedback they give us
> will tell us whether we need to spend more time later implementing a higher
> level toolbar API that can accept multiple components like Button and
> ToggleButton.

I think your assumption here is that implementing such a toolbar is a lot easier, than what's in JEP.
I disagree, all we need is to just split that implementation into two different components toolbars
themself and HTMLView's at the moment toolbar is both. I think it's totally ok if we say Toolbars don't
support buttons in first cut as it will be relatively easy to add such support. But given the way API
is implemented now it wont be possible.

Comment 37

[Dave Townsend [:mossop]]

(In reply to Irakli Gozalishvili [:irakli] [:gozala] [@gozala] from comment #36)
> (In reply to Dave Townsend (:Mossop) from comment #35)
> > We're about to enter our 9th straight month of work on UX APIs. That's too
> > long. Some of that was caused by Australis churn. Too much has been caused
> > by API and implementation churn (something which is as much my
> > responsibility for not clamping down on as anyone else's). I dislike
> > time-bounding implementation work but I have to draw a line somewhere. We
> > need to land something here by the end of the year so we can move on to
> > other things.
> > 
> > Irakli, I think your example is clear but unless HTMLView was implemented
> > and Button and ToggleButton updated with the attachTo code it doesn't
> > actually give us anything functional. 
> 
> Only reason I have not implemented `attachTo` to an actual `Button` and
> `ToogleButton` is
> because Matteo told he was doing some changes to them and I would only cause
> conflicts. Also
> note that only change required for `Button` and `ToogleButton` is that they
> need to inherit
> from general `Component` defined here (so we're talking about few lines
> change):
> https://github.com/mozilla/addon-sdk/pull/1307/files#diff-
> de46563daced2d11639b8d1adbd12d25R16
> 
>  
> Erik's implementation works and gives
> > developers a lot of flexibility to build any kind of toolbar they want. 
> 
> I don't think this is true, that patch is not complete and does not place
> nicely with
> CustomizableUI API provided by Australis.

I'm not sure what impact CustomizableUI has in this case. Toolbars aren't intended to be customisable by the end user to my knowledge.

> > I'm
> > fully aware that that could come back and bite us but I don't see any other
> > option getting implemented and landed in the next 3 weeks.
> >
> 
> While I do understand desire of having toolbar API in 3 weeks, I don't agree
> that
> going with incomplete implementation that doesn't matches either proposed &
> earlier
> discussed API is a reasonable option. I also think that 3 weeks is enough to
> finish
> either implementation.

We need three weeks to completion here, which probably means allowing one week for implementation, one week for review/landing and a spare week for taking care of bugs/overspill.

> > We should solve any obvious problems with the API Erik has created  (I
> > didn't spot any mentioned so far other than it not matching the JEP) and
> > review and land the implementation. 
> 
> Well, I believe we discussed toolbars to large extend and JEP reflects what
> we agreed
> made most sense and avoided all the issues we've in the past. I have not
> went through
> specifics because some of them have being discussed verbally and others are
> in the
> etherpad (which also includes your comments BTW). I feel like we're back to
> square one
> now, so I'll try to summarize why I don't like this API and trying to defend
> what we
> agreed upon earlier:
> 
> 1. This API implies that any kind of toolbar needs full blown HTML document
> presumably in
>    iframe. This also implies that that document would need to communicate
> with add-on somehow
>    presumable through message passing. In nutshell these are same issues
> what made pushed
>    us towards deprecating widgets.

HTMLView would need this too right? However a full iframe for the toolbar gets rid of an awful lot of complexity on our side, like how to size individual items in the toolbar and what to do when we run out of space.

>    If you look at how devs use the widgets it's easy to put them in
> following groups:
>  
>    A) Buttons
>    B) Visuals (graps, weather, etc...)
>    C) Everything else
>    
>    Most of the use cases could have being covered by simple APIs like new
> Buttons & maybe remote
>    canvas. Making message passing a tool for only C use case (which btw
> isn't common).

I disagree. Almost every custom toolbar I've seen includes items that aren't covered by our existing options of Button and ToggleButton. We don't see it much in widgets because they are simple things added to existing toolbars and fight for real estate space.

>    I don't even mention how much costly it is to have iframe with document
> just to render a button
>    or icon, main issues is dev experience.

A single iframe for the toolbar is presumably less costly than one per HTMLView. I agree it is worse than for the case where you are only using buttons.

>    Bigger space of toolbars doesn't actually makes it better, quite the
> contrary. Devs would need
>    to build their buttons in HTML and set listeners then send messages to
> add-on host and loop
>    back, which is most common complain we get.
> 
>    Now if we implement JEP proposal devs could just create buttons and
> handle clicks right from the
>    add-on code. Not to mention that many tab / window / specific states will
> be also taken cared
>    for them. So `HTMLView` will only remain for drawing something advanced.
> I also hope that we would
>    learn from our users and provide more things like canvas that can be
> added to panel that would
>    cover big chunk of use cases and simplify task for our users.

I agree, I would love for us to have this. I don't believe that we can achieve it in 3 weeks though. I do believe that we will learn more about what developers want in toolbars by getting a basic low-level version out there that they can play with.

> 2. This toolbar API is not extendable, so we will have to either do what
> we're doing with widget API
>    or think harder before we make the same mistake. Not to mention that it
> is awkward that other users
>    will be able to drop buttons to other toolbars but not to SDK ones. Or
> that devs could create
>    simple buttons using our APIs but not use them with our toolbars.

Calling it a low level thing and renaming it was a way to avoid the extensibility issue. That said I'm not sure what is different about the current basic implementation and a more advanced toolbar with a single full-width HTMLView in it. Seems like that is a potential upgrade path in the future, if we ever get there.

> 3. This toolbar API is not very portable across platforms, given limitation
> of platform in some cases
>    & hardware it runs in other cases. On the other hand support for toolbars
> with just buttons would
>    be quite visible. I'm not implying to ditch advanced cases that would
> require full blown document,
>    but those cases could be luxury on the more powerful platforms like
> desktop.

It's as portable as the case where a toolbar includes at least one HTMLView right?

> > We should probably call it a
> > low-level/experimental API, maybe call it something like BaseToolbar or
> > HTMLToolbar since I see this as something we can build on later if we think
> > it is worth it. How developers make use of it and the feedback they give us
> > will tell us whether we need to spend more time later implementing a higher
> > level toolbar API that can accept multiple components like Button and
> > ToggleButton.
> 
> I think your assumption here is that implementing such a toolbar is a lot
> easier, than what's in JEP.
> I disagree, all we need is to just split that implementation into two
> different components toolbars
> themself and HTMLView's at the moment toolbar is both. I think it's totally
> ok if we say Toolbars don't
> support buttons in first cut as it will be relatively easy to add such
> support. But given the way API
> is implemented now it wont be possible.

My assumption is that we have a near complete implementation already in the patch Erik requested review on. Changing the implementation to make it easier to support the full API in the future is additional work that I'm not convinced we have time for and I'm not sure whether it would be worth it when I don't know if we will have the time to revisit this in the future.

Comment 38

[Matteo Ferretti [:zer0] [:matteo]]

(In reply to Dave Townsend (:Mossop) from comment #37)

> > > Irakli, I think your example is clear but unless HTMLView was implemented
> > > and Button and ToggleButton updated with the attachTo code it doesn't
> > > actually give us anything functional. 

I believe toolbar in first iteration can be done without any HTMLView component, just with buttons.

> > Erik's implementation works and gives
> > > developers a lot of flexibility to build any kind of toolbar they want. 

> > I don't think this is true, that patch is not complete and does not place
> > nicely with
> > CustomizableUI API provided by Australis.

I agreed with Irakli.

> I'm not sure what impact CustomizableUI has in this case. Toolbars aren't
> intended to be customisable by the end user to my knowledge.

They're customizable targets, I remember when in SF me and Irakli was working / talking about toolbars he had some issues with them, and he needed to discuss with mconley how to solve. Irakli can provide some deeper information I guess.

> > While I do understand desire of having toolbar API in 3 weeks, I don't agree
> > that
> > going with incomplete implementation that doesn't matches either proposed &
> > earlier
> > discussed API is a reasonable option. I also think that 3 weeks is enough to
> > finish
> > either implementation.

> We need three weeks to completion here, which probably means allowing one
> week for implementation, one week for review/landing and a spare week for
> taking care of bugs/overspill.

I also agree with Irakli here. Honestly, between having a toolbar API that is not what it was intended and don't have it soon, but have it as it was intended, I prefer the latter.
Especially because we're not talking of an API that is already landed. In both case, even with the different implementation made by Erik, we could expects bugs or issue – like in any code – so I won't take this as discriminant between the two implementation here proposed. If we were talking about some we have already in our code base, I could understand, but they're both new.

> > > We should solve any obvious problems with the API Erik has created  (I
> > > didn't spot any mentioned so far other than it not matching the JEP) and
> > > review and land the implementation. 
> > 
> > Well, I believe we discussed toolbars to large extend and JEP reflects what
> > we agreed
> > made most sense and avoided all the issues we've in the past. I have not
> > went through
> > specifics because some of them have being discussed verbally and others are
> > in the
> > etherpad (which also includes your comments BTW). I feel like we're back to
> > square one
> > now, so I'll try to summarize why I don't like this API and trying to defend
> > what we
> > agreed upon earlier:
> > 
> > 1. This API implies that any kind of toolbar needs full blown HTML document
> > presumably in
> >    iframe. This also implies that that document would need to communicate
> > with add-on somehow
> >    presumable through message passing. In nutshell these are same issues
> > what made pushed
> >    us towards deprecating widgets.
> 
> HTMLView would need this too right? However a full iframe for the toolbar
> gets rid of an awful lot of complexity on our side, like how to size
> individual items in the toolbar and what to do when we run out of space.

Notice that having iframe in toolbar should be generally avoided, as can be seen in Widgets bugs, that caused a lot troubles to fx-team that worked on Australis. That also why it would be better limit that to a specific component – HTMLView – instead of the whole toolbar, leaving the rest to buttons.
 
> >    If you look at how devs use the widgets it's easy to put them in
> > following groups:
> >  
> >    A) Buttons
> >    B) Visuals (graps, weather, etc...)
> >    C) Everything else
> >    
> >    Most of the use cases could have being covered by simple APIs like new
> > Buttons & maybe remote
> >    canvas. Making message passing a tool for only C use case (which btw
> > isn't common).
> 
> I disagree. Almost every custom toolbar I've seen includes items that aren't
> covered by our existing options of Button and ToggleButton. We don't see it
> much in widgets because they are simple things added to existing toolbars
> and fight for real estate space.

True, but every custom toolbar made in jetpack didn't use any of our API; if they have to switch to some new API, at least, we should provide flexible, good and quite stable APIs. I would avoid to land something just for the sake of landing, if we're not sure about it, or at least if we can't easily change them without any issue.

> >    I don't even mention how much costly it is to have iframe with document
> > just to render a button
> >    or icon, main issues is dev experience.

> A single iframe for the toolbar is presumably less costly than one per
> HTMLView.

I don't see the difference to have a toolbar with one HTMLView, then.

> >    Bigger space of toolbars doesn't actually makes it better, quite the
> > contrary. Devs would need
> >    to build their buttons in HTML and set listeners then send messages to
> > add-on host and loop
> >    back, which is most common complain we get.
> > 
> >    Now if we implement JEP proposal devs could just create buttons and
> > handle clicks right from the
> >    add-on code. Not to mention that many tab / window / specific states will
> > be also taken cared
> >    for them. So `HTMLView` will only remain for drawing something advanced.
> > I also hope that we would
> >    learn from our users and provide more things like canvas that can be
> > added to panel that would
> >    cover big chunk of use cases and simplify task for our users.
> 
> I agree, I would love for us to have this. I don't believe that we can
> achieve it in 3 weeks though. I do believe that we will learn more about
> what developers want in toolbars by getting a basic low-level version out
> there that they can play with.

But this is not a basic low-level version, IMVHO. As far as I can see, it's different from the JEP, and the underling implementation as a big impact on the high level API. Personally, between the two proposals here, I think the one Irakli suggested is the best one, that give to us and to the developers as well less downsides.
Even if we're not going to implement the iframe / HTMLView thing in time, but only the main toolbars API with buttons, I believed is still far better; especially because it gives to us more room for improvement without sacrificing any high level API.

> > 2. This toolbar API is not extendable, so we will have to either do what
> > we're doing with widget API
> >    or think harder before we make the same mistake. Not to mention that it
> > is awkward that other users
> >    will be able to drop buttons to other toolbars but not to SDK ones. Or
> > that devs could create
> >    simple buttons using our APIs but not use them with our toolbars.
> 
> Calling it a low level thing and renaming it was a way to avoid the
> extensibility issue.

I don't see how. The API for the proper implementation will change so much that we'll basically provide a different API.
Said that, if you're saying that we should land any toolbar API anyway, mark them as experimental, put as low level, and adding a huge comment "we're going to have a different version sooner", and there is a good reason for that, ok. Of course, we'll end up with two different toolbar's module at a certain point, but it could be worthy depends by the reason we do so.

> > > We should probably call it a
> > > low-level/experimental API, maybe call it something like BaseToolbar or
> > > HTMLToolbar since I see this as something we can build on later if we think
> > > it is worth it. How developers make use of it and the feedback they give us
> > > will tell us whether we need to spend more time later implementing a higher
> > > level toolbar API that can accept multiple components like Button and
> > > ToggleButton.
> > 
> > I think your assumption here is that implementing such a toolbar is a lot
> > easier, than what's in JEP.
> > I disagree, all we need is to just split that implementation into two
> > different components toolbars
> > themself and HTMLView's at the moment toolbar is both. I think it's totally
> > ok if we say Toolbars don't
> > support buttons in first cut as it will be relatively easy to add such
> > support. But given the way API
> > is implemented now it wont be possible.

Agreed.

> My assumption is that we have a near complete implementation already in the
> patch Erik requested review on. Changing the implementation to make it
> easier to support the full API in the future is additional work that I'm not
> convinced we have time for and I'm not sure whether it would be worth it
> when I don't know if we will have the time to revisit this in the future.

Especially because this reason, the Irakli implementation is more flexible closest to the original JEP targets we had. Therefore, it gives to us more freedom to implement things in the future with less efforts and API changes.

Comment 39

[Dave Townsend [:mossop]]

(In reply to Matteo Ferretti [:matteo] [:zer0] from comment #38)
> (In reply to Dave Townsend (:Mossop) from comment #37)
> 
> > > > Irakli, I think your example is clear but unless HTMLView was implemented
> > > > and Button and ToggleButton updated with the attachTo code it doesn't
> > > > actually give us anything functional. 
> 
> I believe toolbar in first iteration can be done without any HTMLView
> component, just with buttons.

I don't believe that that will be terribly useful to developers

> > > Erik's implementation works and gives
> > > > developers a lot of flexibility to build any kind of toolbar they want. 
> 
> > > I don't think this is true, that patch is not complete and does not place
> > > nicely with
> > > CustomizableUI API provided by Australis.
> 
> I agreed with Irakli.
> 
> > I'm not sure what impact CustomizableUI has in this case. Toolbars aren't
> > intended to be customisable by the end user to my knowledge.
> 
> They're customizable targets, I remember when in SF me and Irakli was
> working / talking about toolbars he had some issues with them, and he needed
> to discuss with mconley how to solve. Irakli can provide some deeper
> information I guess.

Why are they? I don't see customisability mentioned in the JEP or the etherpad. Both me and Jeff think they shouldn't be customisable.

> > > While I do understand desire of having toolbar API in 3 weeks, I don't agree
> > > that
> > > going with incomplete implementation that doesn't matches either proposed &
> > > earlier
> > > discussed API is a reasonable option. I also think that 3 weeks is enough to
> > > finish
> > > either implementation.
> 
> > We need three weeks to completion here, which probably means allowing one
> > week for implementation, one week for review/landing and a spare week for
> > taking care of bugs/overspill.
> 
> I also agree with Irakli here. Honestly, between having a toolbar API that
> is not what it was intended and don't have it soon, but have it as it was
> intended, I prefer the latter.
> Especially because we're not talking of an API that is already landed. In
> both case, even with the different implementation made by Erik, we could
> expects bugs or issue – like in any code – so I won't take this as
> discriminant between the two implementation here proposed. If we were
> talking about some we have already in our code base, I could understand, but
> they're both new.

But we have the implementation for one and not the other.

> > > > We should solve any obvious problems with the API Erik has created  (I
> > > > didn't spot any mentioned so far other than it not matching the JEP) and
> > > > review and land the implementation. 
> > > 
> > > Well, I believe we discussed toolbars to large extend and JEP reflects what
> > > we agreed
> > > made most sense and avoided all the issues we've in the past. I have not
> > > went through
> > > specifics because some of them have being discussed verbally and others are
> > > in the
> > > etherpad (which also includes your comments BTW). I feel like we're back to
> > > square one
> > > now, so I'll try to summarize why I don't like this API and trying to defend
> > > what we
> > > agreed upon earlier:
> > > 
> > > 1. This API implies that any kind of toolbar needs full blown HTML document
> > > presumably in
> > >    iframe. This also implies that that document would need to communicate
> > > with add-on somehow
> > >    presumable through message passing. In nutshell these are same issues
> > > what made pushed
> > >    us towards deprecating widgets.
> > 
> > HTMLView would need this too right? However a full iframe for the toolbar
> > gets rid of an awful lot of complexity on our side, like how to size
> > individual items in the toolbar and what to do when we run out of space.
> 
> Notice that having iframe in toolbar should be generally avoided, as can be
> seen in Widgets bugs, that caused a lot troubles to fx-team that worked on
> Australis. That also why it would be better limit that to a specific
> component – HTMLView – instead of the whole toolbar, leaving the rest to
> buttons.

Why is having a number of different sized iframes in the toolbar better than a single iframe that is the full width of the toolbar?

> > >    If you look at how devs use the widgets it's easy to put them in
> > > following groups:
> > >  
> > >    A) Buttons
> > >    B) Visuals (graps, weather, etc...)
> > >    C) Everything else
> > >    
> > >    Most of the use cases could have being covered by simple APIs like new
> > > Buttons & maybe remote
> > >    canvas. Making message passing a tool for only C use case (which btw
> > > isn't common).
> > 
> > I disagree. Almost every custom toolbar I've seen includes items that aren't
> > covered by our existing options of Button and ToggleButton. We don't see it
> > much in widgets because they are simple things added to existing toolbars
> > and fight for real estate space.
> 
> True, but every custom toolbar made in jetpack didn't use any of our API; if
> they have to switch to some new API, at least, we should provide flexible,
> good and quite stable APIs. I would avoid to land something just for the
> sake of landing, if we're not sure about it, or at least if we can't easily
> change them without any issue.

I'm not talking about toolbars made in jetpack, I'm ttalking about any toolbar made by any exttension.

> 
> > >    I don't even mention how much costly it is to have iframe with document
> > > just to render a button
> > >    or icon, main issues is dev experience.
> 
> > A single iframe for the toolbar is presumably less costly than one per
> > HTMLView.
> 
> I don't see the difference to have a toolbar with one HTMLView, then.

The only different is implementation cost.

> > >    Bigger space of toolbars doesn't actually makes it better, quite the
> > > contrary. Devs would need
> > >    to build their buttons in HTML and set listeners then send messages to
> > > add-on host and loop
> > >    back, which is most common complain we get.
> > > 
> > >    Now if we implement JEP proposal devs could just create buttons and
> > > handle clicks right from the
> > >    add-on code. Not to mention that many tab / window / specific states will
> > > be also taken cared
> > >    for them. So `HTMLView` will only remain for drawing something advanced.
> > > I also hope that we would
> > >    learn from our users and provide more things like canvas that can be
> > > added to panel that would
> > >    cover big chunk of use cases and simplify task for our users.
> > 
> > I agree, I would love for us to have this. I don't believe that we can
> > achieve it in 3 weeks though. I do believe that we will learn more about
> > what developers want in toolbars by getting a basic low-level version out
> > there that they can play with.
> 
> But this is not a basic low-level version, IMVHO. As far as I can see, it's
> different from the JEP, and the underling implementation as a big impact on
> the high level API. Personally, between the two proposals here, I think the
> one Irakli suggested is the best one, that give to us and to the developers
> as well less downsides.
> Even if we're not going to implement the iframe / HTMLView thing in time,
> but only the main toolbars API with buttons, I believed is still far better;
> especially because it gives to us more room for improvement without
> sacrificing any high level API.

I don't think we'd want to ship a toolbar that only allowed buttons on it. Given the choice between shipping the version Erik has written and not shipping any toolbar component at all which would you go with?

> > > 2. This toolbar API is not extendable, so we will have to either do what
> > > we're doing with widget API
> > >    or think harder before we make the same mistake. Not to mention that it
> > > is awkward that other users
> > >    will be able to drop buttons to other toolbars but not to SDK ones. Or
> > > that devs could create
> > >    simple buttons using our APIs but not use them with our toolbars.
> > 
> > Calling it a low level thing and renaming it was a way to avoid the
> > extensibility issue.
> 
> I don't see how. The API for the proper implementation will change so much
> that we'll basically provide a different API.
> Said that, if you're saying that we should land any toolbar API anyway, mark
> them as experimental, put as low level, and adding a huge comment "we're
> going to have a different version sooner", and there is a good reason for
> that, ok. Of course, we'll end up with two different toolbar's module at a
> certain point, but it could be worthy depends by the reason we do so.

I really don't see a big difference in the APIs, but yes just ending up with two implementations was the possibility in my mind.

> > > > We should probably call it a
> > > > low-level/experimental API, maybe call it something like BaseToolbar or
> > > > HTMLToolbar since I see this as something we can build on later if we think
> > > > it is worth it. How developers make use of it and the feedback they give us
> > > > will tell us whether we need to spend more time later implementing a higher
> > > > level toolbar API that can accept multiple components like Button and
> > > > ToggleButton.
> > > 
> > > I think your assumption here is that implementing such a toolbar is a lot
> > > easier, than what's in JEP.
> > > I disagree, all we need is to just split that implementation into two
> > > different components toolbars
> > > themself and HTMLView's at the moment toolbar is both. I think it's totally
> > > ok if we say Toolbars don't
> > > support buttons in first cut as it will be relatively easy to add such
> > > support. But given the way API
> > > is implemented now it wont be possible.
> 
> Agreed.
> 
> > My assumption is that we have a near complete implementation already in the
> > patch Erik requested review on. Changing the implementation to make it
> > easier to support the full API in the future is additional work that I'm not
> > convinced we have time for and I'm not sure whether it would be worth it
> > when I don't know if we will have the time to revisit this in the future.
> 
> Especially because this reason, the Irakli implementation is more flexible
> closest to the original JEP targets we had. Therefore, it gives to us more
> freedom to implement things in the future with less efforts and API changes.

But if we're never going to implement more in the future then that's just wasted implementation time.

Comment 40

[Erik Vold [:erikvold] (please needinfo? me)]

From the meeting today Irakli said (iirc) he could take this over for me to implement the Toolbar({..}).appendChild(HTMLView) api we discussed and was JEP'd except using HTMLView instead of Widget and where HTMLView is basically { url: 'http...' } as far as I understood.  Or another possibility I thought that I heard was merely changing Toolbar({ url: '' }) to be Toolbar({ items: [ { url: '' } ] }) and I'm not sure which or if both will be done.

Comment 41

[Erik Vold [:erikvold] (please needinfo? me)]

Also from the meeting today the main points that I recall being mentioned against first supporting Toolbar({ url: '' }) and then adding support for Toolbar({ url: '' }).appendChild() was that it (ie the appendChild api) would appear as an after thought and that Toolbar({ url: '' }) may not be supportable other platforms such as Fennec.  The later seems unimportant to me because Toolbar({..}).appendChild(HTMLView()) would face the same issue, maybe I missed something there though.

I'm not sure how important it is that appendChild() appear as an after thought.

My only reasons to favor Toolbar({ url: '' }) is that it is easier to use imo for what I imagine will be the typical use case, and I think it's possible to support both apis, albeit not without making appendChild appear as an after thought even it were implemented at the same time as the Toolbar({ url: '' }) signature.

Comment 42

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

(In reply to Erik Vold [:erikvold] [:ztatic] from comment #41)
> Also from the meeting today the main points that I recall being mentioned
> against first supporting Toolbar({ url: '' }) and then adding support for
> Toolbar({ url: '' }).appendChild() was that it (ie the appendChild api)
> would appear as an after thought and that Toolbar({ url: '' }) may not be
> supportable other platforms such as Fennec.  The later seems unimportant to
> me because Toolbar({..}).appendChild(HTMLView()) would face the same issue,
> maybe I missed something there though.
> 
> I'm not sure how important it is that appendChild() appear as an after
> thought.
> 
> My only reasons to favor Toolbar({ url: '' }) is that it is easier to use
> imo for what I imagine will be the typical use case, and I think it's
> possible to support both apis, albeit not without making appendChild appear
> as an after thought even it were implemented at the same time as the
> Toolbar({ url: '' }) signature.

I don't think appendChild is important, what I think is important is that toolbar will be able
to contain multiple items and that `url` won't be any special. So Initial version will just support:

new Toolbar({ items: [new HTMLView({ url: './index.html' })] });

Comment 43

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1307

Pointer to Github pull-request

Comment 44

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Comment on attachment 8344376 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1307

Matteo is implementation is not complete, yet as it lacks test and and HTMLView
component but toolbar is already usable from my manual test, so I've submitted
it for your feedback to hopefully save time later on full review.

Comment 45

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1323

Pointer to Github pull-request

Comment 46

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1324

Pointer to Github pull-request

Comment 47

[Dave Townsend [:mossop]]

Comment on attachment 8346166 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1323

A couple of questions but generally looks fine.

Comment 48

[[github robot]]

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

https://github.com/mozilla/addon-sdk/commit/a93cad4d02d7cefa6d3e8d2c44398dbf30999836
Merge pull request #1324 from Gozala/bug/more-seq@787390

Bug 787390 - Add few more sequencing functions. r=@erikvold

Comment 49

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1329

Pointer to Github pull-request

Comment 50

[[github robot]]

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

https://github.com/mozilla/addon-sdk/commit/0cc808fa6bc63afee3d102ada6b8f43e1fc456e6
Merge pull request #1329 from Gozala/bug/test-runner@787390

Bug 787390 - Stop creating loaders than are never unloaded. r=@jsantell

Comment 51

[[github robot]]

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

https://github.com/mozilla/addon-sdk/commit/aa3666f9fa14b28a1a2985c959794a59dd7bc6a7
Merge pull request #1307 from Gozala/bug/toolbar@787390

Bug 787390 - Toolbar & Frame API implementations. r=@ZER0

Comment 52

[Matteo Ferretti [:zer0] [:matteo]]

I think it could be considered fixed, even with the adjustment we have to do – make the toolbar uncustomizable, align the buttons on the left but the close button.

Feel free to reopen it, in case.