The default bug view has changed. See this FAQ.

Implement the AppButton on toolbars of all tabs

RESOLVED FIXED in Thunderbird 18.0

Status

Thunderbird
Toolbars and Tabs
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: Paenglab, Assigned: Fallen)

Tracking

(Blocks: 1 bug, {ux-consistency})

unspecified
Thunderbird 18.0
ux-consistency
Dependency tree / graph
Bug Flags:
in-testsuite ?

Thunderbird Tracking Flags

(thunderbird17 fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

5 years ago
With landing of bug 650170 the main toolbar has an AppButton and the menubar is hidden. If you switch to a other tab like Chat or Calendar this button is no more accessible and you have to press ALT to show the menubar. For usability it would be better if this button is available on every, or almost every tab toolbar.

- Is it possible to set this on every "third party" toolbar? 
- If yes on which tab types should this button be inserted?

As written above Chat and Calendar should become this button. I think the Add-on tab don't needs this button because I don't see much what we need from this menu for the Add-ons tab.

- If the button is removed from main toolbar, should we then show the button on other toolbars?

I think not, when the user removes the button, he has no use for it. Or if he put it to the tabs-toolbar he has all the time access and don't need it on a other toolbar.

- What, when the user moves the button to the left or the middle of the main toolbar?

If the button is the first or last element in the main toolbar then the button can be set similar on the other toolbars. If the button is set somewhere else in the toolbar, we can't say where he wants the button exactly and we set it at the default position. The user can then move it where he wants it. And this should be saved.
(Reporter)

Updated

5 years ago
Duplicate of this bug: 787527

Updated

5 years ago
Duplicate of this bug: 787527
(Assignee)

Comment 3

5 years ago
Created attachment 659500 [details] [diff] [review]
Fix - v1

This patch takes care by providing a certain type of button that can be added to the toolbar. The extension still needs to add this button to its toolbarpallette, but its just a matter of adding the button via xul, and maybe making it visible by default.

The selector for this button type is the class "button-appmenu". I originally wanted to use the attribute type="appmenu", but in the customize toolbar dialog the type attribute is stripped out to avoid menus showing up there.

I've added the button to chat, but I didn't find anything to migrate the button into the toolbar. For me it magically showed up without migration, but I don't know why. Maybe someone (Florian, Mike?) can take over that step.
Assignee: nobody → philipp
Status: NEW → ASSIGNED
Attachment #659500 - Flags: review?(mconley)
(Reporter)

Comment 4

5 years ago
I tested it without Lightning because of the actual bustage. But it looks good and adds the button to the Chat toolbar.

Only one thing: the button-appmenu needs now a !important for the min-width: 35px; because it's now only a class and other rules override this.
(Reporter)

Comment 5

5 years ago
Created attachment 660571 [details] [diff] [review]
Fix - v1 unbitrotted

Bug 784676 bitrotted Philipp's patch. I've only unbitrotted it and added the !important to the width rules.

Philipp, I hope it's okay I've done this.
Attachment #659500 - Attachment is obsolete: true
Attachment #659500 - Flags: review?(mconley)
Attachment #660571 - Flags: review?(mconley)
(Assignee)

Comment 6

5 years ago
No problem :)
Comment on attachment 660571 [details] [diff] [review]
Fix - v1 unbitrotted

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

Just one question below - otherwise, code looks good.

You've been bit-rotted in chat-messenger-overlay.xul, but it's a trivial fix.

::: mail/base/content/mailWidgets.xml
@@ +2833,5 @@
>    </binding>
> +
> +  <binding id="appmenu-vertical" extends="chrome://global/content/bindings/toolbarbutton.xml#menu-vertical">
> +    <implementation>
> +      <constructor><![CDATA[

If this is empty, do you even need a constructor?
Attachment #660571 - Flags: review?(mconley) → review+
(Reporter)

Comment 8

5 years ago
Created attachment 662275 [details] [diff] [review]
Fix - v1 unbitrotted again

(In reply to Mike Conley (:mconley) from comment #7)
> Comment on attachment 660571 [details] [diff] [review]
> Fix - v1 unbitrotted
> 
> Review of attachment 660571 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Just one question below - otherwise, code looks good.
> 
> You've been bit-rotted in chat-messenger-overlay.xul, but it's a trivial fix.

Fixed again.

> ::: mail/base/content/mailWidgets.xml
> @@ +2833,5 @@
> >    </binding>
> > +
> > +  <binding id="appmenu-vertical" extends="chrome://global/content/bindings/toolbarbutton.xml#menu-vertical">
> > +    <implementation>
> > +      <constructor><![CDATA[
> 
> If this is empty, do you even need a constructor?

This is Philipp's code, so let's him reply.
Attachment #660571 - Attachment is obsolete: true
Attachment #662275 - Flags: review+
(Assignee)

Comment 9

5 years ago
No constructor needed, looks like thats a leftover from testing. Feel free to remove before checkin.
(Reporter)

Comment 10

5 years ago
Created attachment 663828 [details] [diff] [review]
patch for check-in

Patch for check-in with the constructor code removed.

[Approval Request Comment]
This patch gives the AppButton functionality to the chat toolbar and also to Add-ons like Lightning.
Attachment #662275 - Attachment is obsolete: true
Attachment #663828 - Flags: review+
Attachment #663828 - Flags: approval-comm-aurora?
(Reporter)

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/53a7ba30571c
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite?
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 18.0
Attachment #663828 - Flags: approval-comm-aurora? → approval-comm-aurora+
Checked in: https://hg.mozilla.org/releases/comm-aurora/rev/67b687e60287
status-thunderbird17: --- → fixed

Updated

5 years ago
Depends on: 792884, 792849

Updated

5 years ago
Depends on: 795342

Updated

5 years ago
Depends on: 807629
AppButton doesn't work for standalone windows e.g. external .eml files or "open in new window" - Bug 814956 (currently waiting for input of Philipp [:Fallen])

Furthermore, do we have a bug for introducing AppButton to composition window?
Depends on: 814956

Updated

4 years ago
Depends on: 853523

Updated

3 years ago
Blocks: 977028
You need to log in before you can comment on or make changes to this bug.