Closed Bug 1679247 Opened 2 months ago Closed 2 months ago

remove <deck> from EdAdvancedEdit.xhtml

Categories

(Thunderbird :: General, task)

Tracking

(thunderbird_esr78 wontfix, thunderbird84 wontfix)

RESOLVED FIXED
85 Branch
Tracking Status
thunderbird_esr78 --- wontfix
thunderbird84 --- wontfix

People

(Reporter: mkmelin, Assigned: khushil324)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

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.

Attachment #9189745 - Flags: review?(mkmelin+mozilla)
Status: NEW → ASSIGNED
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?

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

Should it use collapsed instead to do that automatically?

collapsed can work, let me try it.

Attachment #9189745 - Attachment is obsolete: true
Attachment #9189745 - Flags: review?(mkmelin+mozilla)
Attachment #9189812 - Flags: review?(mkmelin+mozilla)

collapsed worked here.

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
Attachment #9189812 - Flags: review?(mkmelin+mozilla) → review+
Target Milestone: --- → 85 Branch
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

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/cd6c7748dcbc
remove <deck> from EdAdvancedEdit.xhtml. r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.