STR: 1. Install memchaser (or any other wide widget API using add-on) ER: By default, it shows up in the palette or panel AR: It shows up in the navbar This is really my own fault because I defaulted widgets to the navbar in fixing bug 885015. In any case, ideally we should fix this in the SDK.

I'm a little confused by this bug, I wasn't aware that this was going to be a requirement and I'm not sure how we'd achieve it without making substantial changes to our widget API, which we're trying to deprecate in favour of our new APIs. I thought we had evaluated the widget uses and decided that the number of users of wide widgets was low enough that we didn't care much here.

Dolske?

My recollection was that we wanted to avoid migrating wide widgets when upgrading to Australis (is that what this bug is for?), but I don't remember talking about what should happen if you install such an addon afterwards. I guess that same thing, for consistency (and because having a wide addon in the navbar is pretty terrible UI). But I suppose it matters a bit less then (for not-migration), since the cause-effect is more obvious. IE, if it looks terrible the user can fairly obviously just uninstall whatever they just added. Is this not possible without SDK changes?

(In reply to Justin Dolske [:Dolske] from comment #3) > My recollection was that we wanted to avoid migrating wide widgets when > upgrading to Australis (is that what this bug is for?), but I don't remember > talking about what should happen if you install such an addon afterwards. I > guess that same thing, for consistency (and because having a wide addon in > the navbar is pretty terrible UI). But I suppose it matters a bit less then > (for not-migration), since the cause-effect is more obvious. IE, if it looks > terrible the user can fairly obviously just uninstall whatever they just > added. > > Is this not possible without SDK changes? Not easily, no. And the SDK changes, as I recall, would be fairly trivial. You'd just want to make this code: https://github.com/mozilla/addon-sdk/blob/master/lib/sdk/widget.js#L607 return early in the case where there's no placement yet, and the widget is wide (this is just a property on the widget spec that the SDK can check, if I'm not mistaken). (as is obvious from that link, there are already changes in the SDK to deal with Australis. I'm not sure I understand if/why that is a problem. I agree with Justin that this isn't top priority, though)

Ah ok, that looks easier than I understood it to be. Matteo, yours I think

So, it's not so easier as it looks like. I tried to follow what Gijs said, but I'm not able to make it working easily. I can see two main issue, here: 1. Both panel and palette are not designed to have wide widgets 2. SDK Widget it contains an <iframe/> My first attempt was leaving the widget in the palette, if it was wide. I got several exception because symbiont was null or sdk wasn't able to get a docshell from the widget's iframe, that in fact it wasn't loaded. Plus, the label of the widget was barely visible (is centered depends by the width of the widget, therefore the center of the widget was "outside" the visible area). My second attempt was putting the widget in the panel, if it was wide. Similar situation as above, even if slightly better. The main problem was, I still get some exception, and when I moved the widget (memchaser) in the navbar, it didn't get initialized (the "initializing memchaser" label was still there). In both case, the usability was pretty awkward, so even if we managed to handle some of the error mentioned above, I'm not sure we should proceed in this way. I would propose again my alternatives here, that I mentioned at the beginning when we started to talk about removing the add-on bar: 1. If the add-on has wide widgets, the SDK will create automatically a toolbar for this add-on, and it will adds all the addon's widget there. The toolbar will be hidden by default, and would be possible to make it visible using 'View > Toolbars' menu (the name of the Toolbar will be the add-on's name). 2. Move the SDK widget in the palette if it's wide, but instead of put the iframe just replace it with the label of the widget and the static icon of the add-on. It will becomes "alive" once moved in the navbar / toolbars. Put the SDK widget in the panel should not be allowed – Gijs, I saw something about that in the CustomizableUI, hope this is doable. I'm adding the patch with the attempts I made; other solutions or suggestions are more than welcome.

The thing is, if there are exceptions when things are in the palette or panel, or when they are moved there, that pretty much means that those exceptions are also going to be there if the user decides to move an SDK widget (wide or otherwise). That's Not Good, and it should be fixed regardless. I don't think that should influence the approach we take regarding where we automatically move them initially.

Sorry if I wasn't clear about it: the main issues here are about the usability and experience, not about the exceptions. As I said, "even if we managed to handle some of the error mentioned above, I'm not sure we should proceed in this way". Wide SDK widgets cannot simply be placed in palette or menu panel as is, because neither palette nor the menu panel are designed for wide widgets.

(In reply to Matteo Ferretti [:matteo] [:zer0] from comment #8) > Sorry if I wasn't clear about it: the main issues here are about the > usability and experience, not about the exceptions. As I said, "even if we > managed to handle some of the error mentioned above, I'm not sure we should > proceed in this way". Wide SDK widgets cannot simply be placed in palette or > menu panel as is, because neither palette nor the menu panel are designed > for wide widgets. So what happens right now if you drag a wide SDK widget to the palette on a regular nightly? Why can't we do the same thing for UX? As regards the panel - it has other wide widgets in, that seems to work... if it gets wider than the palette, of course that's problematic. But then, I think we decided there were very very very very few widgets like that, and we should simply max them out at the panel width. Sounds like we should do that and then just make sure that wide SDK widgets behave the same way the other wide widgets do. Some of the refactoring in bug 885579 should help with achieving that goal.

(In reply to :Gijs Kruitbosch from comment #9) > So what happens right now if you drag a wide SDK widget to the palette on a > regular nightly? Why can't we do the same thing for UX? More or less what I described as second solution: only the label is displayed, with a proper size – not the one specify in the widget – and the text is properly centered. That is not happening in the UX branch. Additionally to the Nightly, we should find a way to prevent that the wide SDK widget can be added to the menu panel – that Nightly doesn't have – as I described above. Is it possible? > As regards the panel - it has other wide widgets in, that seems to work... I tried with memchaser and a test widget made for this purpose, and it didn't work well. My suggestion is avoiding wide widgets in panel – I talked with UX, it seems they want to add only square widget in the panel anyway when is possible, so I think this approach is inline with their view points. > if it gets wider than the palette, of course that's problematic. But then, I > think we decided there were very very very very few widgets like that, and > we should simply max them out at the panel width. Sounds like we should do > that and then just make sure that wide SDK widgets behave the same way the > other wide widgets do. Some of the refactoring in bug 885579 should help > with achieving that goal. Not sure I'm following you here. However, my goal is just be sure that widgets like memchaser wouldn't be broke, so my suggestion is still the same of the 2nd solution I proposed; I would avoid the panel if it's possible.

(In reply to Matteo Ferretti [:matteo] [:zer0] from comment #10) > (In reply to :Gijs Kruitbosch from comment #9) > > > So what happens right now if you drag a wide SDK widget to the palette on a > > regular nightly? Why can't we do the same thing for UX? > > More or less what I described as second solution: only the label is > displayed, with a proper size – not the one specify in the widget – and the > text is properly centered. That is not happening in the UX branch. This sounds like something that would need to be fixed on the SDK side. > Additionally to the Nightly, we should find a way to prevent that the wide > SDK widget can be added to the menu panel – that Nightly doesn't have – as I > described above. Is it possible? I still don't think this is a good idea, but it's possible. A general solution to restrict areas was discussed and binned in bug 873066. You could either reopen the discussion there, or you could implement something yourself using a CustomizableUI listener that returns the items to their original place if they're moved to the menu panel. I still think you should mark any widget that's wider than the square widgets and smaller than the width of the panel as a "wide" widget separately, and let people put it in the panel. > > As regards the panel - it has other wide widgets in, that seems to work... > > I tried with memchaser and a test widget made for this purpose, and it > didn't work well. Define "didn't work well"? How wide was your test widget?

(In reply to :Gijs Kruitbosch from comment #11) > > > So what happens right now if you drag a wide SDK widget to the palette on a > > > regular nightly? Why can't we do the same thing for UX? > > More or less what I described as second solution: only the label is > > displayed, with a proper size – not the one specify in the widget – and the > > text is properly centered. That is not happening in the UX branch. > This sounds like something that would need to be fixed on the SDK side. Why? It's a regression on Australis about what is currently happening on regular Nigthly; I believe that UX branch should be at least behave in this scenario as Nightly does. If this regression is caused somehow by SDK sure, it should be fixed there, but I can't imaging anything on SDK that can bring this difference. It seems more that something is changed in the style of the palette. Anyway, at least is worthy to investigate on UX branch side why there is this difference. > > Additionally to the Nightly, we should find a way to prevent that the wide > > SDK widget can be added to the menu panel – that Nightly doesn't have – as I > > described above. Is it possible? > I still don't think this is a good idea, but it's possible. A general > solution to restrict areas was discussed and binned in bug 873066. You could > either reopen the discussion there, or you could implement something > yourself using a CustomizableUI listener that returns the items to their > original place if they're moved to the menu panel. Notice that widget are going to be deprecated, so this solution is temporary: the fact that old API / UI element, that are going to disappear, can't use new UI thing (menu panel) but only the old things (toolbar, palette) seems reasonable to me. Unfortunately (SDK) widgets were designed badly at the beginning, and we do not have a good control on their size or appearance, so they don't scale really well on panel, that is not designed for such things, bring a bad usability to the user. We could of course makes the widget in the panel square as it was in the palette, but in this such case it would be "dead", where the elements in the menu panel are meant to be "alive". That was already discussed with UX, but we can reopen that discussion if you want, and looking for better solutions. > I still think you should mark any widget that's wider than the square > widgets and smaller than the width of the panel as a "wide" widget > separately, and let people put it in the panel. What is the width of the panel? What should be happen to the (SDK) widget that are wider than the panel? > > > As regards the panel - it has other wide widgets in, that seems to work... > > > > I tried with memchaser and a test widget made for this purpose, and it > > didn't work well. > > Define "didn't work well"? How wide was your test widget? I mentioned already before what I meant: bad usability and exceptions. Where we can definitely fix the last one, the first one is coming from the wide nature of the widget. You can test by yourself using memchaser (you can download here: https://addons.mozilla.org/En-us/firefox/addon/memchaser/). The width seems to be 360px. Another reason because I would personally avoid the widget in the panel is because the performance: they are iframe in toolbaritem, that can load complex HTML and remote one too: restrict them to the palette and toolbar seems better to my, but it's just my opinion. :) I will gather some other feedback, maybe others in jetpack have different idea / opinion about that.

(In reply to Matteo Ferretti [:matteo] [:zer0] from comment #12) > (In reply to :Gijs Kruitbosch from comment #11) > > > > > So what happens right now if you drag a wide SDK widget to the palette on a > > > > regular nightly? Why can't we do the same thing for UX? > > > > More or less what I described as second solution: only the label is > > > displayed, with a proper size – not the one specify in the widget – and the > > > text is properly centered. That is not happening in the UX branch. > > > This sounds like something that would need to be fixed on the SDK side. > > Why? It's a regression on Australis about what is currently happening on > regular Nigthly; I believe that UX branch should be at least behave in this > scenario as Nightly does. If this regression is caused somehow by SDK sure, > it should be fixed there, but I can't imaging anything on SDK that can bring > this difference. It seems more that something is changed in the style of the > palette. Anyway, at least is worthy to investigate on UX branch side why > there is this difference. We no longer use a separate window for the palette. I just checked, and I see memchaser being initialized (ie "alive" in your usage below) in the palette, rather than "dead", i.e. just a label. That's not something that can be fixed in Firefox, it needs to be fixed in the SDK code. > > > Additionally to the Nightly, we should find a way to prevent that the wide > > > SDK widget can be added to the menu panel – that Nightly doesn't have – as I > > > described above. Is it possible? > > > I still don't think this is a good idea, but it's possible. A general > > solution to restrict areas was discussed and binned in bug 873066. You could > > either reopen the discussion there, or you could implement something > > yourself using a CustomizableUI listener that returns the items to their > > original place if they're moved to the menu panel. > > Notice that widget are going to be deprecated, so this solution is > temporary: the fact that old API / UI element, that are going to disappear, > can't use new UI thing (menu panel) but only the old things (toolbar, > palette) seems reasonable to me. Unfortunately (SDK) widgets were designed > badly at the beginning, and we do not have a good control on their size or > appearance, so they don't scale really well on panel, that is not designed > for such things, bring a bad usability to the user. > We could of course makes the widget in the panel square as it was in the > palette, but in this such case it would be "dead", where the elements in the > menu panel are meant to be "alive". That was already discussed with UX, but > we can reopen that discussion if you want, and looking for better solutions. Your point that the old API shouldn't be available in the new UI things like the menu panel sounds to me like you're aiming to wean devs off this API by 'punishing' use of it by not having it work as well as others. The problem is this solution also punishes users: it's not clear to a user why they can move most things wherever, but somehow this widget is "special", and can feel like a bug in Firefox to them. I'd rather the widget could move to the panel, and then didn't look great (meaning the user can file bugs against the add-on). I don't understand the "we do not have a good control on their size" bit. The appearance I get - this is up to the add-on dev, as far as I'm concerned, and we should trust them to update their stuff when Australis gets closer to release. The size, I was under the impression you controlled the width and height of the iframe, and that these are available to you at widget construction time because they are part of the widget's options/spec as passed to the widgets module. Is this not true? If it is, that sounds like we could make a decision to show some widgets in the panel (ie those that fit) and not others (ie those that do not fit). > > I still think you should mark any widget that's wider than the square > > widgets and smaller than the width of the panel as a "wide" widget > > separately, and let people put it in the panel. > > What is the width of the panel? What should be happen to the (SDK) widget > that are wider than the panel? ~20em, IIRC. Check in DOMI what this boils down to in px if necessary. Wider widgets we could restrict to toolbars, or even just mark incompatible on AMO as far as I'm concerned, but check what other folks think about this. AIUI this is a tiny minority of SDK add-ons anyway (per the research Jeff did a few months back). > > > > > As regards the panel - it has other wide widgets in, that seems to work... > > > > > > I tried with memchaser and a test widget made for this purpose, and it > > > didn't work well. > > > > Define "didn't work well"? How wide was your test widget? > > I mentioned already before what I meant: bad usability and exceptions. Where > we can definitely fix the last one, the first one is coming from the wide > nature of the widget. > You can test by yourself using memchaser (you can download here: > https://addons.mozilla.org/En-us/firefox/addon/memchaser/). The width seems > to be 360px. So memchaser is too wide. If you use a wide widget that's smaller, and give it the wide widget class, how does that work? https://mxr.mozilla.org/projects-central/source/ux/browser/components/customizableui/src/PanelWideWidgetTracker.jsm#26 - although I suspect we may change this class still, it's a bit of a misnomer currently.

So, when we looked at this in the spring we concluded that user impact wasn't the right argument to use for anything because there are very few add-ons that use wide widgets, and those add-ons aren't very popular. In my opinion wide SDK widgets shouldn't be in the navbar, but I don't want to spend a lot of time on this issue because the user impact will be low. I feel bad that there is no place in the new UI for memchaser as-is, but I can't see a reason to engineer around it.

(In reply to :Gijs Kruitbosch from comment #13) > (In reply to Matteo Ferretti [:matteo] [:zer0] from comment #12) > > (In reply to :Gijs Kruitbosch from comment #11) > > > > > > > So what happens right now if you drag a wide SDK widget to the palette on a > > > > > regular nightly? Why can't we do the same thing for UX? > > > > > > More or less what I described as second solution: only the label is > > > > displayed, with a proper size – not the one specify in the widget – and the > > > > text is properly centered. That is not happening in the UX branch. > > > > > This sounds like something that would need to be fixed on the SDK side. > > > > Why? It's a regression on Australis about what is currently happening on > > regular Nigthly; I believe that UX branch should be at least behave in this > > scenario as Nightly does. If this regression is caused somehow by SDK sure, > > it should be fixed there, but I can't imaging anything on SDK that can bring > > this difference. It seems more that something is changed in the style of the > > palette. Anyway, at least is worthy to investigate on UX branch side why > > there is this difference. > > We no longer use a separate window for the palette. I just checked, and I > see memchaser being initialized (ie "alive" in your usage below) in the > palette, rather than "dead", i.e. just a label. That's not something that > can be fixed in Firefox, it needs to be fixed in the SDK code. Is there something special about the SDK added toolbar items that causes this problem? Why wouldn't any toolbar item added by an extension have exactly the same issues when in the panel?

(In reply to Matteo Ferretti [:matteo] [:zer0] from comment #12) > (In reply to :Gijs Kruitbosch from comment #11) > > > > > So what happens right now if you drag a wide SDK widget to the palette on a > > > > regular nightly? Why can't we do the same thing for UX? > > > > More or less what I described as second solution: only the label is > > > displayed, with a proper size – not the one specify in the widget – and the > > > text is properly centered. That is not happening in the UX branch. > > > This sounds like something that would need to be fixed on the SDK side. > > Why? It's a regression on Australis about what is currently happening on > regular Nigthly; I believe that UX branch should be at least behave in this > scenario as Nightly does. If this regression is caused somehow by SDK sure, > it should be fixed there, but I can't imaging anything on SDK that can bring > this difference. It seems more that something is changed in the style of the > palette. Anyway, at least is worthy to investigate on UX branch side why > there is this difference. We're all on team Firefox here. It's irrelevant whether this is a regression or not. The only question is where is the best place to put the code fix. If this is something that affects add-ons in general then front-end Firefox probably makes sense. If it is only SDK code then fixing it in widget land is probably better rather than complicating the frontend code to cover a case that we're deprecating.

(In reply to Dave Townsend (:Mossop) from comment #16) ... > ...If it is only SDK code then fixing it in widget land > is probably better rather than complicating the frontend code to cover a > case that we're deprecating. In particular, we could patch widget.js to looks for values of width >== $some_number and handle these differently, like clipping their visibility to some sane size. Jorge can include this issue in the ( sure to be mammoth ) Australis add-ons compatibility post, developers will adjust and update their code, etc.

(In reply to Dave Townsend (:Mossop) from comment #16) > We're all on team Firefox here. It's irrelevant whether this is a regression > or not. The only question is where is the best place to put the code fix. If > this is something that affects add-ons in general then front-end Firefox > probably makes sense. If it is only SDK code then fixing it in widget land > is probably better rather than complicating the frontend code to cover a > case that we're deprecating. So 'plain' toolbarbuttons don't really have problems here. The issue is with <toolbaritem>s or, in some select cases, with toolbarbuttons that have custom bindings (like the downloads button). AFAICT we've always solved the interactivity of toolbaritems with more complicated structures during customization mode individually, e.g. the bookmarks toolbar items ( http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.css#134 ), the downloads button ( http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.css#697 ) and the search bar binding ( http://mxr.mozilla.org/mozilla-central/source/browser/components/search/content/search.xml#70 ). Note that this is all existing code on m-c. In regular Nightly, you can actually also see that memchaser reinitializes in the toolbar when you start customizing. Ideally, that shouldn't happen, and it should display only its label (like it does in the customization dialog in regular nightly, and like e.g. the bookmarks toolbar items). The reason it does that in the customization panel is probably because the customization panel will itself create a new "dummy" item to be shown in this panel, and just moves the original item to the (hidden) toolbox palette. In UX, we no longer do that, and just move the existing item to a visible palette that's in the same document. This explains the discrepancy we're seeing. The way this works has traditionally required add-ons to also overlay the customization dialog if they needed to do anything special. That's no longer necessary in UX. It seems that some authors will have to do a bit more work, if they didn't already deal with customize mode in a way that disabled their widgets, and others will have to do a bit less (if they already had code for this but had to specialcase the customization dialog because it was a different document, needed its own overlay, etc.). Because Firefox frontend code can't really know what's inside toolbaritems, or how best to make them not do anything except showing a label, I don't think we should try to fix this in frontend code.

(In reply to :Gijs Kruitbosch from comment #18) > The way this works has traditionally required add-ons to also overlay the > customization dialog if they needed to do anything special. That's no longer > necessary in UX. It seems that some authors will have to do a bit more work, > if they didn't already deal with customize mode in a way that disabled their > widgets, and others will have to do a bit less (if they already had code for > this but had to specialcase the customization dialog because it was a > different document, needed its own overlay, etc.). So what is the work that we can do here? How can the add-on developer know whether their widget has been moved to the panel or back. With the customize dialog it's easy with overlays etc. but I don't know how we'd do it here.

(In reply to Dave Townsend (:Mossop) from comment #19) > (In reply to :Gijs Kruitbosch from comment #18) > > The way this works has traditionally required add-ons to also overlay the > > customization dialog if they needed to do anything special. That's no longer > > necessary in UX. It seems that some authors will have to do a bit more work, > > if they didn't already deal with customize mode in a way that disabled their > > widgets, and others will have to do a bit less (if they already had code for > > this but had to specialcase the customization dialog because it was a > > different document, needed its own overlay, etc.). > > So what is the work that we can do here? How can the add-on developer know > whether their widget has been moved to the panel or back. With the customize > dialog it's easy with overlays etc. but I don't know how we'd do it here. The SDK add-on developer shouldn't need to do anything, the SDK should fix this for them. Non-SDK add-on developers can know (both with/without Australis) because the wrapper (toolbarpaletteitem) in which the item is put has a place attribute indicating where it is (toolbar/palette/panel). They'd just style accordingly (display: none the complicated context, display: -moz-box/block the placeholder content - like the bookmarks toolbar items I linked earlier). We also expose events that the SDK could use, because it apparently has 0 stylesheets, and 0 ways of loading stylesheets. :-( At the moment it looks like I'll end up fixing this particular part of the issue as part of bug 931092.

(In reply to :Gijs Kruitbosch from comment #20) > (In reply to Dave Townsend (:Mossop) from comment #19) > > (In reply to :Gijs Kruitbosch from comment #18) > > > The way this works has traditionally required add-ons to also overlay the > > > customization dialog if they needed to do anything special. That's no longer > > > necessary in UX. It seems that some authors will have to do a bit more work, > > > if they didn't already deal with customize mode in a way that disabled their > > > widgets, and others will have to do a bit less (if they already had code for > > > this but had to specialcase the customization dialog because it was a > > > different document, needed its own overlay, etc.). > > > > So what is the work that we can do here? How can the add-on developer know > > whether their widget has been moved to the panel or back. With the customize > > dialog it's easy with overlays etc. but I don't know how we'd do it here. > > The SDK add-on developer shouldn't need to do anything, the SDK should fix > this for them. Non-SDK add-on developers can know (both with/without > Australis) because the wrapper (toolbarpaletteitem) in which the item is put > has a place attribute indicating where it is (toolbar/palette/panel). They'd > just style accordingly (display: none the complicated context, display: > -moz-box/block the placeholder content - like the bookmarks toolbar items I > linked earlier). Great, this is exactly what I was looking for. > We also expose events that the SDK could use, because it apparently has 0 > stylesheets, and 0 ways of loading stylesheets. :-( Excellent. I don't think there is anything stopping us from loading stylesheets, we've just never needed them before. > At the moment it looks like I'll end up fixing this particular part of the > issue as part of bug 931092. Ok, good to know

Gijs, with the fix in bug 931092 and bug 959640, is there anything left to do here?

(In reply to Matteo Ferretti [:matteo] [:zer0] from comment #22) > Gijs, with the fix in bug 931092 and bug 959640, is there anything left to > do here? Newly installed wide SDK widgets (just tried with the version of memchaser on AMO) now still show up in the navbar. Ideally I'd like that not to happen.

It seems that now the issue I faced the last time are no more, probably due the massive changes we had recently in SDK and probably i australis too.

Commits pushed to master at https://github.com/mozilla/addon-sdk https://github.com/mozilla/addon-sdk/commit/9fe778b17bd3ecad9c297870e61521166b6bd6b9 Bug 894390 - Australis: Wide SDK widgets shouldn't be in the nav-bar https://github.com/mozilla/addon-sdk/commit/7d71adc312f6ee3ffb08c58803c00e8cf1129926 Merge pull request #1440 from ZER0/wide-widget/894390 fix Bug 894390 - Australis: Wide SDK widgets shouldn't be in the nav-bar r=@Mossop

This should fixes the tree; my only concerns is I don't know a better way to check if the wide widget is actually in the palette, so I just added a test to check that a wide widget is not actually in the Firefox UI, like a regular widget it is.

Commits pushed to master at https://github.com/mozilla/addon-sdk https://github.com/mozilla/addon-sdk/commit/d88ba09fd88b2cf56539b45b1af9f92c71543607 Bug 894390 - Australis: Wide SDK widgets shouldn't be in the nav-bar - fixed tests https://github.com/mozilla/addon-sdk/commit/c4560bc49038866a971b435444aa19288385e10c Merge pull request #1444 from ZER0/fix-widget-test/894390 Bug 894390 - Australis: Wide SDK widgets shouldn't be in the nav-bar r=@Mossop