252468 - (autostretch) Remove outdated autostretch attribute from the tree

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Simon Paquet [:sipaq]]

This is a followup bug for bug 94470.

Comment 1

[Simon Paquet [:sipaq]]

Attachment: Patch v1

Comment 2

[Boris Zbarsky [:bzbarsky]]

rs=me once this has sr=neil

Comment 3

[[e-mail]]

Comment on attachment 153906 [details] [diff] [review]
Patch v1

As well as the cases where elements have no children there are a couple of
cases where you don't need autostretch or align: you need
*) an hbox inside a vbox, or vice versa
*) the inner box must not have flex
*) an inner vbox may only have one child, except for <spacer>
*) an inner hbox must have only one child tag, and/or <spacer>
But don't change any you aren't happy with.

>-    box.setAttribute("autostretch", "never");
You don't seem to replace this for some reason...

>-            <row autostretch="never" valign="middle">
>+            <row align="center" valign="middle">
While you're doing cleanup, valign="middle" was the default (with
autostretch="never"); align="center" covers both..

Comment 4

[Simon Paquet [:sipaq]]

Attachment: Patch v2

Patch addressing review comments

Comment 5

[Simon Paquet [:sipaq]]

Comment on attachment 154588 [details] [diff] [review]
Patch v2

rs=bz, since this has been review+'ed by Neil (see comment 2).

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

checked in. please mark fixed if it is.

Comment 7

[Simon Paquet [:sipaq]]

Seems like I overlooked one more instance of autostretch in the code.

See
http://lxr.mozilla.org/seamonkey/source/mailnews/compose/resources/content/mailComposeBindings.xml#9


Neil, can we just replace this with align="center" again or can this be removed?

Comment 8

[[e-mail]]

(In reply to comment #7)
>can we just replace this with align="center" again or can this be removed?
It can be removed ;-) let me dig out my patch for that...

Comment 9

[[e-mail]]

Attachment: Remove mailComposeBindings.xml

Comment 10

[[e-mail]]

Comment on attachment 155574 [details] [diff] [review]
Remove mailComposeBindings.xml

Similar changes will apply to mail/, do you want me to write a combined patch?

Comment 11

[Simon Paquet [:sipaq]]

Neil, I would never have dared to completely eliminate the whole file, but I
like your change :-)

Shouldn't you also ask for sr or is mailnews strongly owned now and a single
review is sufficient?

Comment 12

[[e-mail]]

Comment on attachment 155574 [details] [diff] [review]
Remove mailComposeBindings.xml

Silly me, this is bug 244886.

Comment 13

[Christian :Biesinger (don't email me, ping me on IRC)]

looks like the firefox part needs relanding

Comment 14

[Ian Neal]

Relanding toolkit part of patch v2 following landing of aviary branch