Bug 1414406 removed the ability of (legacy) TB add-ons to show and "Options" button on the Add-ons Manage page. I tested a 14th Nov and 16th Nov Daily and the latter doesn't show "Options" buttons any more since bug 1414406 landed on 15th Nov.

So, the API is this install.rdf part: <em:optionsURL>chrome://tabswitch/content/options.xul</em:optionsURL> This means the host application should give the user some way to access the Addon's Options. Once the user clicked on that button or menu item, the host application should open a window or tab where it loads the XUL URL given by the addon. There's another install.rdf property: <em:optionsType>2</em:optionsType> The options type is defined as: Options will be opened in a new window OPTIONS_TYPE_DIALOG: 1, // Options will be displayed within the AM detail view OPTIONS_TYPE_INLINE: 2, // Options will be displayed in a new tab, if possible OPTIONS_TYPE_TAB: 3, // Same as OPTIONS_TYPE_INLINE, but no Preferences button will be shown. // Used to indicate that only non-interactive information will be shown. OPTIONS_TYPE_INLINE_INFO: 4, // Similar to OPTIONS_TYPE_INLINE, but rather than generating inline // options from a specially-formatted XUL file, the contents of the // file are simply displayed in an inline <browser> element. OPTIONS_TYPE_INLINE_BROWSER: 5, That's a lot of options, but I would consider them rather a wish of the addon, not a requirement that we implement all that. We only need to implement those that are required for the addon to actually work, and that are also commonly used. For example, if an addon says "inline", but we open it in a new window, and it works anyway, that's fine. That's a lot better than just breaking it entirely. I would recommend to unsupport the inline type and simply show it in a new window, if that's working. I don't know whether the Addon Options dialog also relies on a special API (functions) that is exposed in the Options dialog. If that's the case, we'd also need to implement these functions. However, I don't see evidence of that. We might need to send a few events to the inline dialogs, e.g. "domwindowopened" and "domwindowclosed", for them to know that they need to initialize now and close now, but that shouldn't be too hard. Here's the code that Gecko removed to support Addon Options: https://reviewboard.mozilla.org/r/198822/diff/5#index_header

Do we have stats how many addons use this feature? A grep through the addons source DB would show.

From bug 1396172 comment #21: === It turns out that it is actually bug 1418914 that is preventing from a WebExtension being used for in-line preferences. Resurrecting the xbl stuff might be an option, but I suspect isn't sustainable, and may be hard to hook into the add-ons manager (especially now the support for the install.rdf options is also removed, so you can't install an add-on declaring the old-style inline preferences). ===

I was looking at the change in bug 1414406 and it seems optionsURL is still supported but you need to specify the optionsType in the addon's install.rdf. The only supported option for extensions seems to be 3 (OPTIONS_TYPE_TAB). So if the optionsURL points to a proper XUL file in the addon, this would be enough (just opens a tab instead of a dialog with the xul). When I added this attribute (as <em:optionsType>) into an addon, the Preferences/Options button appeared in the Addonmanager besides the addon. So there is still support. Clicking the button does nothing, because it calls https://dxr.mozilla.org/comm-central/rev/ba84e01c50eed07f85e3ced37eaaf134843e2a6a/mozilla/toolkit/mozapps/extensions/content/extensions.js#1555 and we miss https://dxr.mozilla.org/comm-central/rev/ba84e01c50eed07f85e3ced37eaaf134843e2a6a/mozilla/browser/base/content/browser.js#8309 . Any way we could copy/access this to c-c? We probably can't save OPTIONS_TYPE_INLINE with it's special XUL syntax, but proper XUL files opened in a dialog (OPTIONS_TYPE_DIALOG was the default when optionsURL was set) could be doable.

Summarising: The add-on has, for example: <em:optionsURL>chrome://lightningcalendartabs/content/options.xul</em:optionsURL> Currently that shows no button. As suggested, I added <em:optionsType>3</em:optionsType> and lo and behold, the button now shows! So the next point is going to make it do something, and that's where we need to implement switchToTabHavingURI() from M-C's browser.js somewhere in mail/. I can look into that, thanks for the investigation!

I tried <em:optionsType>1</em:optionsType> to open the options in a new windows and that gives the following error: This add-on could not be installed because it appears to be corrupt. So opening in a tab is the only option. We could open it in a new window instead, but that would be somewhat funny since the option asks for a tab. Should we land this as a temporary or permanent fix and tell add-on authors to add option "3" to their add-ons? I tried with "Lightning Calendar Tabs" and I can change its preference without problem.

We could do some more work to find the tab if it's already open. Actually, how to you iterate the open tabs and query them?

Try some functions in mail/base/content/tabmail.xml .

For the record: OPTIONS_TYPE_DIALOG was removed here: https://hg.mozilla.org/mozilla-central/rev/ac82533933fb#l6.13

Lightning Calendar Tabs working and with options button configured to be opened in a tab. Attached for testing purposes.

A side experiment. This adds a menu "addon preferences" in the Tools options where it exposes all addons that have a optionsURL set. It opens the URL in a dialog if clicked. This has the advantage to bypass the hurdles that the addon manager may create in the future (e.g. whether it even exposes the Preferences buttons and when). It ignores the optionsType (addons wouldn't need to change). We could actually open a dialog if optionsType is missing and a tab if optionsType==3.

Actually, we could use both patches. For addons that migrate to optionsType==3, jorg's patch would trigger and Preferences button would appear and it would open in a tab, obeying the value. For addons that won't migrate (many won't) and will not have optionsType set, only those could be listed in my proposed list and would open in a dialog (which was the default till now).

Comment on attachment 8937179 [details] [diff] [review] WIP experiment Review of attachment 8937179 [details] [diff] [review]: ----------------------------------------------------------------- Nice-ö, but add-on is with a hyphen. I'd add both solutions. ::: mail/base/content/mailWindowOverlay.js @@ +3731,5 @@ > + getService().currentThread; > + while (!done) { > + dump("ACE: pumping ...\n"); > + thread.processNextEvent(true); > + } Is that needed? ::: mail/base/content/mailWindowOverlay.xul @@ +3017,5 @@ > key="key_savedFiles" > oncommand="openSavedFilesWnd();"/> > <menuitem id="addonsManager" label="&addons.label;" accesskey="&addons.accesskey;" > oncommand="openAddonsMgr();"/> > + <menu id="addonsManager_prefs" label="Addon preferences" oncommand="openAddonPrefs(event.target.value);"> OK, this is going to be a localisable string.

So there would be open questions: 1. should this menu list only addons without optionsType, or also addons with optionsType==3 (and open a tab for them)? 2. where should the menuitem be placed in our menus? Under the Add-ons item in Tools as I have it, or under Preferences/Options (which are in different menus on Windows and Linux). 3. where to put this in appmenu?

Oh, something's wrong with the Lightning options, I get a stand-alone window with the Advanced options.

Comment on attachment 8937179 [details] [diff] [review] WIP experiment This is a good idea. I have not many extensions in my test profile. With this patch, Lightning has it's own pref dialog ;) Compact Headers is also shown but doesn't show a usable dialog. There is also no optionsType defined. My before Tabs Toolbar uses inline options with optionsType 2 and show no dialog. Maybe only extensions should be shown which have optionsType 1 set. All others should be ignored. Would it be possible to overlay this prefs to the options buttons of the extensions? We have already a overlay for the navigation buttons. A problem is also, that extensions with other than optionsType 3 can't be installed.

The Compact headers look good with the actual Daily but with my build which has bug 1379338 applied it breaks. So only optionsType 2 should be ignored.

(In reply to Richard Marti (:Paenglab) from comment #16) > With this patch, Lightning has it's own pref dialog ;) Yes :) > My before Tabs Toolbar uses inline options with optionsType 2 and show no > dialog. Yes, inline options are not supported and we can't fix this. > Maybe only extensions should be shown which have optionsType 1 set. All > others should be ignored. No, with optionsType==1 the addon can't even be installed. > Would it be possible to overlay this prefs to the options buttons of the > extensions? We have already a overlay for the navigation buttons. > A problem is also, that extensions with other than optionsType 3 can't be > installed. Yes. But with empty optionsType they can, so this menuitem would support old addons that do not have any optionsType. As apparently it defaulted to 1 (dialog) so it worked without the attribute. (In reply to Richard Marti (:Paenglab) from comment #17) > The Compact headers look good with the actual Daily but with my build which > has bug 1379338 applied it breaks. So only optionsType 2 should be ignored. Actually we must ignore everything except empty optionsType or a value of 3 (tab) as we can't support the other values. That is what the patch does.

Opening Lightning in a tab actually resizes the whole TB window to a small dialog ;) So Lightning would need some tweaks to work in a tab. Yes, it actually opens inside the main Preferences dialog, which is special and will probably not work with any of the patches.

(In reply to :aceman from comment #14) > So there would be open questions: > 1. should this menu list only addons without optionsType, or also addons > with optionsType==3 (and open a tab for them)? You gave already the answer. > 2. where should the menuitem be placed in our menus? Under the Add-ons item > in Tools as I have it, or under Preferences/Options (which are in different > menus on Windows and Linux). I think whereit is now near to the Add-ons menuitem is good. > 3. where to put this in appmenu? Also directly after the Add-ond menuitem? (In reply to :aceman from comment #19) > Opening Lightning in a tab actually resizes the whole TB window to a small > dialog ;) So Lightning would need some tweaks to work in a tab. Yes, it > actually opens inside the main Preferences dialog, which is special and will > probably not work with any of the patches. Lightning should be excluded.

Sorts addons by name in the menu and adds addon icons :)

(In reply to Richard Marti (:Paenglab) from comment #20) > (In reply to :aceman from comment #14) > > So there would be open questions: > > 1. should this menu list only addons without optionsType, or also addons > > with optionsType==3 (and open a tab for them)? > > You gave already the answer. And what is it? Yes I can display both, the question is if we want it. We could let optionsType==3 addons run through the m-c code and jorg's patch. On the other hand, having all addon prefs at one place would be nice. I can also let optionsType==3 addons open in a tab using jorg's function so the display would be identical. It is already prepared in WIP v2.

(In reply to :aceman from comment #21) > Created attachment 8937202 [details] [diff] [review] > WIP experiment v2 > > Sorts addons by name in the menu and adds addon icons :) I like it. On Windows, Mac not tested, no icons are shown. Could you add class="menuitem-iconic"? Lightning should be not shown as it's also only a partial prefs dialog. Or open the main prefs with the Calendar tab. I think also optionsType==3 should be shown, and then opened in a tab. I thought a bit about the AppMenu. Would it be possible to show the Add-ons menuitem like it is now, when no extension pref exists. And when there are extension prefs, make a splitmenu and show the prefs in this menu?

(In reply to :aceman from comment #8) > Try some functions in mail/base/content/tabmail.xml . Hmm, I've looked for a while but didn't see anything. I didn't even find a spot where tab URLs are stored so I could search whether there is already a tab for that URL. If you open a mail message multiple times, you get multiple tabs and no one has complained about this so far.

I think that if you give a tab a name and then try to open a new tab with the same name, it opens/updates the existing tab with that name… At least that's how it works in Firefox.

Right, I've just tried my patch (attachment 8937133 [details] [diff] [review]) with the modified "Lightning Calendar Tabs" and yes, it doesn't open the same tab twice. So the patch is good to go as it is.

(In reply to Jorg K (GMT+1) from comment #24) > (In reply to :aceman from comment #8) > > Try some functions in mail/base/content/tabmail.xml . > Hmm, I've looked for a while but didn't see anything. I didn't even find a > spot where tab URLs are stored so I could search whether there is already a > tab for that URL. If you open a mail message multiple times, you get > multiple tabs and no one has complained about this so far. I see, but there is shouldSwitchTo() implemented for a particular tab type, which says whether openTab should actually switch to an existing tab (and which one). Maybe we should create a new tab type for these addon preferences. But if not, maybe you could use some code from contentTabBaseType::shouldSwitchTo() in your function to find the right tab and not open a new one.

(In reply to Richard Marti (:Paenglab) from comment #23) > I like it. On Windows, Mac not tested, no icons are shown. Could you add > class="menuitem-iconic"? I add this class. Strange that it is not working in Win. It means addon.iconURL is null? Can it return different values on different platforms? > Lightning should be not shown as it's also only a > partial prefs dialog. Or open the main prefs with the Calendar tab. Strangely, I reshuffled the code now and Calendar opens now in the full Preferences window. It just doesn't focus Calendar tab directly. > I think also optionsType==3 should be shown, and then opened in a tab. OK. > I thought a bit about the AppMenu. Would it be possible to show the Add-ons > menuitem like it is now, when no extension pref exists. And when there are > extension prefs, make a splitmenu and show the prefs in this menu? Deciding whether to show a splitmenu (with right arrow) or just a menuitem would need to enumerate the addons every time the menu is opened. The enumeration may be expensive for many addons. I wouldn't want to do it before the user wants to really open the addon prefs submenu. Can we have a splitmenu in all cases, but the submenu would be empty? Or have a placeholder saying "no addon settings found"?

(In reply to :aceman from comment #28) > (In reply to Richard Marti (:Paenglab) from comment #23) > > I like it. On Windows, Mac not tested, no icons are shown. Could you add > > class="menuitem-iconic"? > > I add this class. Strange that it is not working in Win. It means > addon.iconURL is null? Can it return different values on different platforms? The standard on Mac and Windows is not to show icons in menus. With class="menuitem-iconic" the menuitem becomes the ability to show icons. > Deciding whether to show a splitmenu (with right arrow) or just a menuitem > would need to enumerate the addons every time the menu is opened. The > enumeration may be expensive for many addons. I wouldn't want to do it > before the user wants to really open the addon prefs submenu. Can we have a > splitmenu in all cases, but the submenu would be empty? Or have a > placeholder saying "no addon settings found"? Create two items, one normal, as we have now, and a second with the splitmenu, and then decide on popupshow which one should be shown is slow too? If yes, okay with always showing the splitmenu.

(In reply to Richard Marti (:Paenglab) from comment #29) > > I add this class. Strange that it is not working in Win. It means > > addon.iconURL is null? Can it return different values on different platforms? > > The standard on Mac and Windows is not to show icons in menus. With > class="menuitem-iconic" the menuitem becomes the ability to show icons. I already do that in the current patch as it is needed for Linux too: + if (iconURL) { + item.setAttribute("class", "menuitem-iconic"); + item.setAttribute("image", iconURL); > Create two items, one normal, as we have now, and a second with the > splitmenu, and then decide on popupshow which one should be shown is slow > too? If yes, okay with always showing the splitmenu. I think that is the same. How do I find out which item to show, without enumerating all addons and checking if any one of them has prefs? Yes, in this case I can bail out on the first one that has prefs. The question is if we can cache this decision. Can the list of active addons change in a session? Yes, but only with restartless addons. Can those have prefs (optionsURL) that we need to show in the menu?

This version adds: 1. appmenu support 2. optionsType == 3 (in a tab) are exposed too and opened in a tab, not a dialog. 2. Lightning prefs seem to work correctly (I just don't understand why). 3. restartless addons have optionsURL too so they are not excluded (tried with "Restart" addon 4. if 32/48px icon (iconURL) is not found, use also 64px one. May it be none of your addons had an icon that is why you see none in your Windows profile? But Lightning should definitely have an icon.

Comment on attachment 8937852 [details] [diff] [review] WIP experiment v3 Looks good, thanks! The icons are shown now. And we should use "Add-on Preferences" (uppercase P) on Linux/Mac and "Add-on Options" on Windows like they are used for the buttons in Add-on Manager. When no extensions have a pref or are disabled, no popup is shown in main menu and on AppMenu a empty rectangle is shown. Didn't you want to add a "No Add-on Options/Preferences found".

Comment on attachment 8937852 [details] [diff] [review] WIP experiment v3 Nice. This needs the other patch (attachment 8937133 [details] [diff] [review]) if the add-on has tab options. You could just merge the two patches and add the code required not to open the same tab twice ;-) Lightning still opens the overall preferences and the Calendar tab isn't activated. So that needs more polishing, or drop Lightning from the menu.

Lightning also doesn't change to its tab when opened from Add-on Manager in TB 52. This needs to be addressed in Lightning itself.

(In reply to Richard Marti (:Paenglab) from comment #34) > Lightning also doesn't change to its tab when opened from Add-on Manager in > TB 52. This needs to be addressed in Lightning itself. Yes, the optionsURL of Lightning is chrome://messenger/content/preferences/preferences.xul which opens the generic TB preferences dialog. I don't think it has a way to pass the argument saying it wants the Calendar tab to be focused. We can pass that argument when opening preferences from various places in the UI. When no argument is passed, I think the dialog reopens the last used position in the dialog (panel/tab). The addon manager only passes the URL when clicking the Preferences button (see https://dxr.mozilla.org/comm-central/rev/7cb064f0d25e9de7c77589a9459da1ade502b4bb/mozilla/toolkit/mozapps/extensions/content/extensions.js#1533). If calendar wants to ensure the Calendar pane is opened, it needs to find a way, maybe something like chrome://messenger/content/preferences/preferences.xul#calendar could work (or have code added in the preferences dialog to read and interpret the anchor). (In reply to Richard Marti (:Paenglab) from comment #32) > And we should use "Add-on Preferences" (uppercase P) on Linux/Mac and > "Add-on Options" on Windows like they are used for the buttons in Add-on > Manager. OK > When no extensions have a pref or are disabled, no popup is shown in main > menu and on AppMenu a empty rectangle is shown. Didn't you want to add a "No > Add-on Options/Preferences found". Yes, I wanted if we can't find other way. My comment 31 answers my question from comment 30, that restartless addons can change the list of addons at runtime and can have optionsURL so need to be added to my list dynamically. So caching isn't much help. (In reply to Jorg K (GMT+1) from comment #33) > Nice. This needs the other patch (attachment 8937133 [details] [diff] [review] > [review]) if the add-on has tab options. You could just merge the two > patches and add the code required not to open the same tab twice ;-) I hoped you can finish that part in your patch and then we have 2 patches that nicely complement each other :)

(In reply to :aceman from comment #35) > I hoped you can finish that part in your patch and then we have 2 patches > that nicely complement each other :) In fact, the three lines I added were based on your analysis in comment #4. Please add them to your patch. I have other building sites to worry about.

This should be almost final. 1. adds the placeholder to say if there are no addons with prefs to show in the menu. 2. adds the code to not open the same pref tab twice. 3. distiguishes the options/preferences strings for Win/Unix. The session even restores the tab with prefs if you reopen TB. The last thing weird to me is why is the switchToTabHavingURI() put into mailWindow.js. Jorg, was there any reason for the placement? Didn't it work elsewhere? E.g. I have the openAddonPrefs() in mailCore.js .

(In reply to :aceman from comment #37) > Jorg, was there any reason for the placement? None.

Comment on attachment 8938169 [details] [diff] [review] 1419145.patch Looks good, thanks!

Comment on attachment 8938169 [details] [diff] [review] 1419145.patch Review of attachment 8938169 [details] [diff] [review]: ----------------------------------------------------------------- ::: mail/base/content/mailWindowOverlay.js @@ +3739,5 @@ > + while (!done) { > + thread.processNextEvent(true); > + } > + > + // Populate the menu with addon names and icons Can you move this into the callback to avoid the ugly wait loop. And remove the trailing space ;-)

As far as I can see, this works, too. Also remove the trailing space. What are we going to do with restartless add-ons? Can they have options pages? I guess so.

Comment on attachment 8938516 [details] [diff] [review] 1419145.patch - no wait Review of attachment 8938516 [details] [diff] [review]: ----------------------------------------------------------------- Yes, this seems to work. I would have expected a visual effect of seeing the menu being populated after opening, but I can't notice it even with slowed down CPU. So maybe it doesn't happen and the 'wait' is done implicitly by some other mechanism. Please add the ui-r into the commit message.

(In reply to Jorg K (GMT+1, mostly AFK 22-27 Dec., bustage-fix only) from comment #41) > What are we going to do with restartless add-ons? Can they have options > pages? I guess so. Yes, I said restartless addons should be included, they have options dialogs too (I have one).

Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/f45975e98daa Expose add-on preferences via a menu. For addons with optionsType==3, load the prefs into a new tab. r=jorgk, ui-r=Paenglab

Thank you for doing the work to fix this situation.

If I understand this correctly this is removing modeless Dialogs from xul completely, which is a very bad idea. I have UI changing preferences for a toolbar that can only be seen on the 3pane window - this is effectively breaking the automatic preview as forcing all options into its own tab removes this context. I also had another addon (quickPasswords) taking its context from the current tab. Is there a way to have modeless windows in a different way?

I don't know what 'modeless' means, but we still support addon prefs in a dialog/window. That is what this bug is about. But you must have no optionsType field in the addon's install.rdf. If you have that field (and want the Options button to be shown in the Addon manager), then the only allowed value is 3, which yes, is 'open in tab'.

> I don't know what 'modeless' means modeless = non-modal: a dialog that allows access to the owner / parent window while "floating on top". It is not restricted to the parent windows client area. modal dialog: blocks access to the parent (e.g. MessageBox) Adding: <?xml-stylesheet href="chrome://messenger/skin/preferences/preferences.css" type="text/css" ?> into the options dialogs fixes the broken dialog UI. I also had some problems with a breaking line in chrome manifest: content quickfolders-pl content/ platform removing that line fixes most of the Add-on behavior. ONe additional problem at the moment is the fact that browser toolbox console source links don't work (which makes it hard to debug files like chrome.manifest as it is not clear which add-on is meant) My restartless MenuOnTop (which hoisted the menu above the mail tabs) is yet to be fixed, hopefully the browser toolbox will help here.

(In reply to Axel Grude [:realRaven] from comment #48) > ONe additional problem > at the moment is the fact that browser toolbox console source links don't > work (which makes it hard to debug files like chrome.manifest as it is not > clear which add-on is meant) Sadly that's broken in Daily after 2018-02-17. We're looking into it, bug 1439021. Can you use a Daily of 2018-02-16?

This isn't working for me on Mac, tested with 59.0b2 and 60.0b1 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:59.0) Gecko/20100101 Thunderbird/59.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:60.0) Gecko/20100101 Thunderbird/60.0 - Create new clean profile - Look at menu Tools - Add-on Preferences - Expected to see preferences for Lightning, but it says "No Add-on settings found." - Dismiss menu then look at it again - This time there is no submenu at all. - Install addons such as "CompactHeader", restart - Results same as above, first time "No Add-on settings found", second time submenu is missing.

Richard, could you please try on Mac.

Yes, I see this too on Mac. The AppMenu works but the main menu not. No error in console.

So how can Aceman fix this without having a Mac?

Paenglab could add debugging output in the initAddonPrefsMenu() function to see which line it fails at.

Ace, could you add a patch with this code? Then I can test it.

This returns us to Aceman's initial idea of "1419145.patch" of waiting for the result of the add-ons manager. I think we need to do this since there appears to be a timing issue on Mac. Richard, can you please try this. r+ since it was Aceman's original idea. If you need bootstrapped add-ons with options menu, use: https://addons.mozilla.org/en-US/thunderbird/addon/single-domain/?src=cb-dl-updated Or I can provide more if you need.

It's easier to see what happens in a "no white-space" version.

Comment on attachment 8964315 [details] [diff] [review] 1419145-follow-up.patch This works. I have only one extension in this profile which shows but can't be started because it's a no-bootstrapped one. But before it didn't show in tools menu but in AppMenu. Now it's shown in both.

Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/90deaec87201 Follow-up: Wait for add-on manager to return before building menu. r=me DONTBUILD

Beta (TB 60 beta 2): https://hg.mozilla.org/releases/comm-beta/rev/0d6ed7a480beb8c00dc7b573f967265d433847d4 This will get it fixed in TB 60 beta 2 for Mac users.

Added test for the new submenu in bug 1462923.