Add Add-ons Options menu item to the Menu button

RESOLVED FIXED in Thunderbird 59.0

Status

defect
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: walts48, Assigned: aceman)

Tracking

Thunderbird 59.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

3.12 KB, patch
aceman
: review+
Details | Diff | Splinter Review
Reporter

Description

a year ago
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:59.0) Gecko/20100101 Firefox/59.0
Build ID: 20171229220051

Steps to reproduce:

After bug 1419145 landed there is a menu item under Tools in the Menu bar, titled Add-ons Options.
Selecting it opens a sub-menu to the right of the > with a list of extensions with preferences.
This is missing from the Menu button.


Actual results:

Using the Menu button and selecting Add-ons shows a list of extensions with preferences, but it blocks access to opening the Add-ons manager.


Expected results:

I think a separate Add-ons Options menu item should be listed under the Menu button, like it is for Tools in the Menu bar.
Users will go to select Add-ons in the Menu button, and be confused as to why they are seeing a list of extensions.
Reporter

Updated

a year ago
OS: Unspecified → All
Hardware: Unspecified → All

Comment 1

a year ago
Thanks for filing the bug!
Flags: needinfo?(acelists)
Assignee

Updated

a year ago
Assignee: nobody → acelists
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(acelists)
Assignee

Comment 2

a year ago
Posted patch 1427407.patch (obsolete) — Splinter Review
This could do it. Adds the add-ons as the first item in the submenu, as seems the style (and necessity) also in other such <splitmenu>s in the appmenu.
Attachment #8939162 - Flags: ui-review?(richard.marti)
Attachment #8939162 - Flags: review?(richard.marti)
Comment on attachment 8939162 [details] [diff] [review]
1427407.patch

Review of attachment 8939162 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!
Attachment #8939162 - Flags: ui-review?(richard.marti)
Attachment #8939162 - Flags: ui-review+
Attachment #8939162 - Flags: review?(richard.marti)
Attachment #8939162 - Flags: review+

Updated

a year ago
Keywords: checkin-needed

Comment 4

a year ago
Comment on attachment 8939162 [details] [diff] [review]
1427407.patch

Review of attachment 8939162 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/base/content/mailWindowOverlay.js
@@ +3715,5 @@
>   *  @option aMenupopup  The menupopup element to populate.
>   */
>  function initAddonPrefsMenu(aMenupopup) {
> +  // Clear all but the special menuitems on top.
> +  let noPrefsElem = aMenupopup.querySelector('[disabled="true"]');

Actually, I'm looking at this and I don't understand it.
The comment says: Clear all but the special menuitems on top.
So it refers to a plural. So which items are they?
Are they always disabled? I thought the "no add-ons here" is sometimes enabled.
The variable holds a single reference, so that doesn't match the comment.
Attachment #8939162 - Flags: review?(jorgk)

Updated

a year ago
Keywords: checkin-needed
Assignee

Comment 5

a year ago
(In reply to Jorg K (GMT+1) from comment #4)
> Actually, I'm looking at this and I don't understand it.
> The comment says: Clear all but the special menuitems on top.
> So it refers to a plural. So which items are they?
In the main menu, there is only one special item (the "no add-on prefs" one, disabled), in the appmenu there are 3:
add-on manager, menuseparator, the "no add-on prefs" item (disabled).

The loop is removing element bottom up and we need it to stop on the special items. So I based the check on hitting the disabled=true item, which is the "no add-on prefs", currently.

> Are they always disabled? I thought the "no add-ons here" is sometimes
> enabled.

It is never enabled (you can't click it), it is just collapsed (hidden) as needed.

> The variable holds a single reference, so that doesn't match the comment.

Yes, it is the single element that is the last one of the special ones. On this boundary the clearing loop has to stop.

Comment 6

a year ago
(In reply to :aceman from comment #5)
> The loop is removing element bottom up and we need it to stop on the special
> items. So I based the check on hitting the disabled=true item, which is the
> "no add-on prefs", currently.
OK, very tricky, could you document this, please.

Something like:
// Starting at the bottom, clear all menu items until we hit
// "no add-on prefs" which is disabled. On the app menu there could be
// further items above it like a separator and the add-ons manager entry.
Assignee

Comment 7

a year ago
Thanks, done.
Attachment #8939162 - Attachment is obsolete: true
Attachment #8939162 - Flags: review?(jorgk)
Attachment #8939172 - Flags: review+

Comment 8

a year ago
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/2639c3ea8ba0
add "Add-ons" menuitem in the Add-ons submenu of appmenu. ui-r=Paenglab, r=Paenglab
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED

Updated

a year ago
Target Milestone: --- → Thunderbird 59.0
Depends on: 1428082
Reporter

Comment 9

a year ago
This is still confusing.

The user uses the Menu Button > Add-ons, and after a slight delay sees a list of extensions with no understanding of why they are there.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 10

a year ago
This is shipping now in TB 59 beta, so no point to leave the bug open. In fact, my query didn't find the bug when I was looking for bugs that need to go into the release notes :-(

If you're not happy with the solution, please file another bug.
Status: REOPENED → RESOLVED
Last Resolved: a year agoa year ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.