1508236 - remove broadcasters from mail/components/compose

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, task)

Description

[Magnus Melin [:mkmelin]]

+++ This bug was initially created as a clone of Bug #1489447 +++

Remove <broadcaster> from mail/components/compose

Comment 1

[Magnus Melin [:mkmelin]]

Attachment: bug1508236_de_broadcaster_compose.patch

The <broadcaster id="args" value="editorType=default"/> I don't know what's it about. Seems fine without it...

I moved the functional per-platform stylings to the shared folder, and renamed the .css file at the same time (overlay is silly now).

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=c8693bbe868b6ab639b4a2caad9438dc56bcb7ec

Comment 2

[:aceman]

Comment on attachment 9026367 [details] [diff] [review]
bug1508236_de_broadcaster_compose.patch

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

Thanks, the toggling of Contacts sidebar seems to work.
I'd like Paenglab to check the css changes.
Also you remove style for signing="notok" and crypto="notok", what is the replacement?

::: mail/components/compose/content/MsgComposeCommands.js
@@ +6226,5 @@
>    if (sidebarBox.hidden) {
>      // Show contacts sidebar.
>      sidebarBox.hidden = false;
>      sidebarSplitter.hidden = false;
> +    document.getElementById("menu_AddressSidebar").setAttribute("checked", "true");

Make a variable for document.getElementById("menu_AddressSidebar"), as the other elements have.

@@ +6228,5 @@
>      sidebarBox.hidden = false;
>      sidebarSplitter.hidden = false;
> +    document.getElementById("menu_AddressSidebar").setAttribute("checked", "true");
> +    if (document.getElementById("button-contacts")) {
> +      document.getElementById("button-contacts").disabled = true;

document.getElementById("button-contacts") is used 4x you could make a variable for it, as the other elements have.

Comment 3

[:aceman]

Comment on attachment 9026367 [details] [diff] [review]
bug1508236_de_broadcaster_compose.patch

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

::: mail/components/compose/content/MsgComposeCommands.js
@@ +6226,5 @@
>    if (sidebarBox.hidden) {
>      // Show contacts sidebar.
>      sidebarBox.hidden = false;
>      sidebarSplitter.hidden = false;
> +    document.getElementById("menu_AddressSidebar").setAttribute("checked", "true");

Make a variable for document.getElementById("menu_AddressSidebar"), as the other elements have.

@@ +6228,5 @@
>      sidebarBox.hidden = false;
>      sidebarSplitter.hidden = false;
> +    document.getElementById("menu_AddressSidebar").setAttribute("checked", "true");
> +    if (document.getElementById("button-contacts")) {
> +      document.getElementById("button-contacts").disabled = true;

document.getElementById("button-contacts") is used 4x you could make a variable for it, as the other elements have.

::: mail/components/compose/content/messengercompose.xul
@@ +1005,5 @@
>            <menuitem id="menu_AddressSidebar"
>                      label="&addressSidebar.label;" accesskey="&addressSidebar.accesskey;"
> +                    type="checkbox"
> +                    key="key_addressSidebar"
> +                    oncommand="toggleAddressPicker();"/>

Why not migrating the "autoCheck" ?

Comment 4

[:aceman]

Comment on attachment 9026367 [details] [diff] [review]
bug1508236_de_broadcaster_compose.patch

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

Actually, it seems this has also lost 'checked' on 'button-contacts' which made a depressed effect when the sidebar was open. I thought that removal was intentional and the attribute unused, but it does not seem so. Also, it was possible to click the button again to close the sidebar, which feature you also dropped.

Comment 5

[Richard Marti (:Paenglab)]

Comment on attachment 9026367 [details] [diff] [review]
bug1508236_de_broadcaster_compose.patch

Yes, the deactivating of the "Contacts" button is unusual because then it's not possible to hide the sidebar again with the button. Checked is a better feedback than disabled.

Comment 6

[Magnus Melin [:mkmelin]]

(In reply to :aceman from comment #2)
> Also you remove style for signing="notok" and crypto="notok", what is the
> replacement?

They were never set to that (during composition) which this css covers. During composition all that the icons in the status bar display is really whether you have chosen that you will encrypt and/or sign.

Comment 7

[Magnus Melin [:mkmelin]]

Attachment: bug1508236_de_broadcaster_compose.patch

Comment 8

[:aceman]

Comment on attachment 9027699 [details] [diff] [review]
bug1508236_de_broadcaster_compose.patch

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

Thanks.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/fbf6d186a18a
Remove broadcasters from mail/components/compose. r=aceman