1679247 - remove <deck> from EdAdvancedEdit.xhtml

Status: RESOLVED FIXED

(Thunderbird :: General, task)

Description

[Magnus Melin [:mkmelin]]

Remove <deck> from EdAdvancedEdit.xhtml and js
https://searchfox.org/comm-central/rev/f8c11a36225bbe5e145f1a9a0a3801428e265715/mail/components/compose/content/dialogs/EdAdvancedEdit.xhtml#90-103

There are only two children of the deck. Instead of a deck, they could just as well be set to hidden while the other is not, and vice versa.
Shouldn't be any sizing effects since this one has size from the parent.

Comment 1

[Khushil Mistry [:khushil324]]

Attachment: Bug-1679247_de-dack-EdAdvancedEdit-xhtml-0.patch

Comment 2

[Magnus Melin [:mkmelin]]

Comment on attachment 9189745 [details] [diff] [review]
Bug-1679247_de-dack-EdAdvancedEdit-xhtml-0.patch

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

::: mail/components/compose/content/dialogs/EdAEHTMLAttributes.js
@@ +198,3 @@
>            // Switch to using editable menulist
> +          gDialog.AddHTMLAttributeValueMenulist.parentElement.hidden = false;
> +          gDialog.AddHTMLAttributeValueTextbox.hidden = true;

why hid this if we're still going to hide the parent next?

Comment 3

[Khushil Mistry [:khushil324]]

The parent element is the XUL element and child is <html:input>. So hiding the XUL parent element is not hiding the <html:input>.

Comment 4

[Magnus Melin [:mkmelin]]

Should it use collapsed instead to do that automatically?

Comment 5

[Khushil Mistry [:khushil324]]

collapsed can work, let me try it.

Comment 6

[Khushil Mistry [:khushil324]]

Attachment: Bug-1679247_de-dack-EdAdvancedEdit-xhtml-0.patch

Comment 7

[Khushil Mistry [:khushil324]]

collapsed worked here.

Comment 8

[Magnus Melin [:mkmelin]]

Comment on attachment 9189812 [details] [diff] [review]
Bug-1679247_de-dack-EdAdvancedEdit-xhtml-0.patch

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

LGTM, r=mkmelin

Comment 9

[Magnus Melin [:mkmelin]]

Comment on attachment 9189812 [details] [diff] [review]
Bug-1679247_de-dack-EdAdvancedEdit-xhtml-0.patch

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

::: mail/components/compose/content/dialogs/EdAEHTMLAttributes.js
@@ +194,4 @@
>        if (listLen > 1) {
>          gDialog.AddHTMLAttributeValueMenulist.removeAllItems();
>  
> +        if (attributeValueMenulistCollapsed) {

actually, looks like this is not really needed. I'll update the patch slightly

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/cd6c7748dcbc
remove <deck> from EdAdvancedEdit.xhtml. r=mkmelin