70857 - [XUL Syntax] Replace <boxDerivedTag align="horizontal|vertical"> with <boxDerivedTag orient="horizontal|vertical">

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[David Hyatt]

Using align to specify orientation has been deprecated for a year. Let's
ditch it and make sure we've replaced all uses of it with orient.

Comment 1

[Ben Goodger (use ben at mozilla dot org for email)]

or, better yet, hbox & vbox.

Comment 2

[[e-mail]]

There are quite a few aligns out there!
http://lxr.mozilla.org/seamonkey/search?string=align%3D%22horizontal%22
http://lxr.mozilla.org/seamonkey/search?string=align%3D%22vertical%22

Comment 3

[Blake Ross]

*** This bug has been marked as a duplicate of 73671 ***

Comment 4

[David Hyatt]

Nope, blake, this isn't a dup.  I've updated the summary to make it more clear.

In many places, people say things like...

<someTag align="vertical">

where they can't replace it with <vbox>, since <someTag> isn't necessarily a box.

Comment 5

[Mark Anderson]

I think I've got this done locally....  I'll attach a couple of patches I've 
come up with today.  It just brutally changes every instance in CSS and XUL of 
align="horizontal" and align="vertical" to orient="horizontal" and 
orient="vertical", respectively.  The only files I couldn't do are in the Mac 
classic skin (which I have no means of getting ahold of, unless I hijack my 
parents' Mac).  Could there be any fallout from this change, or are they 
equivalent in every way (thus meaning I couldn't break anything with this)? :)

Comment 6

[Mark Anderson]

Attachment: Patch to the files packaged in jars, sans Mac Classic (and maybe Win Classic, although it should have patched that, too)

Comment 7

[Mark Anderson]

Attachment: Patch to the files in the res directory (non-jarred)

Comment 8

[Mark Anderson]

Attachment: New patch, encompassing the entire tree (including Mac Classic)

Comment 9

[Mark Anderson]

I got a complete tree and ran a cvs diff against it.  That's the most recent 
patch.  It should be ready for r=/sr=, assuming there's nothing that these 
changes could possibly mess up (I have no resources to regression test 
everything).

Comment 10

[Mark Anderson]

Attachment: Patch for this bug, bug 70748, and bug 65428

Comment 11

[Mark Anderson]

The patch following (ignore the first four) does the following things:

Bug 70857:
All instances of align="horizontal" -> orient="horizontal" in XUL and CSS
All instances of align="vertical" -> orient="vertical" in XUL and CSS

Bug 70748:
All instances of <titledbox> -> <groupbox> in all files (and all fallout 
therein, including the change of the atom)
New ugly styling of <titledbox> put in to encourage migration, at suggestion of 
ben

Bug 65428:
All instances of type="text/javascript" -> type="application/x-javascript"
All instances of language="javascript" -> type="application/x-javascript"

No bug I know of:
<html:script> -> <script> in XUL (I was in the files anyway, so I just thought, 
what the heck)

Should all be there now.  Thanks for your patience with me. :)

Comment 12

[Mark Anderson]

Attachment: Correct patch (all problems stated solved)

Comment 13

[Mark Anderson]

I'll take this one.  Targeting to 0.9.3, assuming I can get r=/sr= on the patch 
I'll submit later.  The new one fixes all previously mentioned issues, plus 
73671.

Comment 14

[Mark Anderson]

Attachment: Proposed fix for all mentioned changes and bugs

Comment 15

[Mark Anderson]

Attachment: Patch that should (knock on wood) be ready for r=/sr=

Comment 16

[Mark Anderson]

This last patch contains the correct fix for 70748 (it's a lot less work than I 
was fearing), and it has been tested somewhat more.  Also, I forgot to mention 
earlier that this also fixes a minor bug in Mac Classic (the CSS styles for the 
change to <label> instead of <title> had not been changed for Mac Classic's 
titledbox.css).  So, this should be good to go, and it should work completely 
(but I have no way to regression test everything, of course), unless I've made a 
minor mistake in conversion to <hbox> and <vbox> (the only thing I'm worried 
about right now, because my converter that I wrote may have missed a fringe 
case).  Also, <titledbox> and <groupbox> are both supported for now, per Ben's 
concerns on IRC (<titledbox> is merely replaced in all instances in Mozilla; it 
has not yet been deprecated).

I of course have no way to alter Netscape's commercial files that may have 
instances of the older syntaxes, so I'm going to have to count on someone else 
to do that.  But no existing instances should be broken by any of these changes, 
as support has not been removed for any syntax as of yet.

Comment 17

[Mark Anderson]

Attachment: Patch with r=timeless, ready for sr=

Comment 18

[Mark Anderson]

This latest patch has r=timeless (given on IRC) and is awaiting sr=blake. :)

Comment 19

[Blake Ross]

Will look into sr'ing tomorrow.

Comment 20

[Mark Anderson]

Ok, the following patch is sr=blake for all except one more fix that I added, 
which corrects <vbox> grippy orientation in Modern and Mac Classic (at request 
of rginda); it had been different from <box orient="vertical">.  That should be 
sr=blake by the end of tonight.  This forthcoming patch is brought up to the 
tip, and is ready for blake to checkin right at the opening of 0.9.3, whatever 
god you may believe in willing.

Comment 21

[Mark Anderson]

Attachment: Hopefully, the last patch this bug'll need, diffed to the tip of 0.9.2

Comment 22

[Mark Anderson]

Ok.  Let's get this landed, shall we?  Cc:ing [e-mail] so he'll get the 
notification of the patch I'm attaching right after this message.  It's diffed 
to the trunk as of 2:30 PDT.  It has r=timeless and sr=blake.  It just needs jj 
to run some tests, and we can carpool it and get this monster out of the way.

Comment 23

[Mark Anderson]

Attachment: Patch, updated to the trunk as of 2:30 PDT today

Comment 24

[Mark Anderson]

Attachment: Final patch, with the one bug fixed (ready for carpool)

Comment 25

[Mark Anderson]

Attachment: Ok. Let's carpool this one, today. Updated for the Modern changes from last night

Comment 26

[Simon Fraser [no longer active]]

That last patch didn't apply cleanly on a pull I just did. The following files 
falied:
1 out of 1 hunk FAILED when patching os2/platformDialogOverlay.xul
1 out of 8 hunks FAILED when patching EdAdvancedEdit.xul
2 out of 3 hunks FAILED when patching EdImageMap.xul
1 out of 8 hunks FAILED when patching EdImageProps.xul
1 out of 2 hunks FAILED when patching EdLinkProps.xul

Comment 27

[Mark Anderson]

Attachment: Patch that's absolutely ready for carpool (as said in an email last night)

Comment 28

[J.J. Enser]

Mark, carpool starts, you're on. Please land your changes now and let me know 
when you're done so that I can kick off verification builds

Comment 29

[Mark Anderson]

Attachment: New patch for blake (using diff -u, but not -w)

Comment 30

[Mark Anderson]

Ok, so I was for some reason thinking that -w on diff actually made it consider 
whitespace.  Oops.  Stupid me. :)

Anyway, here's the new patch blake asked for (should apply just fine to a clean 
tree).

Comment 31

[Mark Anderson]

Attachment: Refresh, because of Chatzilla and prefs dialog changes

Comment 32

[Mark Anderson]

Attachment: Smaller CSS/JS/XBL patch, to be checked in to allow XUL changes to go in easier

Comment 33

[timeless]

attachment 41364 [details] [diff] [review] checked in. no conflicts.

Comment 34

[Mark Anderson]

Attachment: XUL Changes, to land today

Comment 35

[Mark Anderson]

Attachment: Minor CSS changes (align="vertical" deprecation will be part of another bug)

Comment 36

[timeless]

checked in everything but security ...

Comment 37

[Mark Anderson]

Attachment: Security diff

Comment 38

[David P. Drinan]

Security changes checked in.

Comment 39

[Mark Anderson]

Woohoo!

Comment 40

[Mark Anderson]

See also bug 89999 for final changes, once commercial is handled.

Comment 41

[David :Bienvenu]

this fix seems to have caused bug 90055, if anyone involved in this bug wants to
look at that, that would be nice.

Comment 42

[(not reading, please use [e-mail] instead)]

this landing has broken several things in mailnews.

folder pane, account manager, filters, reply, etc.

how'd we go from "Patch that's absolutely ready for carpool" to a patch that 
has horked mailnews?   did it ever not hork mailnews?

Comment 43

[(not reading, please use [e-mail] instead)]

bug #90068 tracks the mailnews horkage.

the rest of the product may be horked as well.

Comment 44

[[e-mail]]

Classic splitter CSS still uses window/box[align="vertical"] :-(

Comment 45

[Mark Anderson]

Right.  That CSS change is in bug 89999.  I'm gonna let it simmer for a little 
while (and commercial still needs to update anyway, and I'd care not to have 3 
dozen Bugscape bugs to my name, too).

(And, for those who care, the mailnews situation is under control, thanks to 
some great work by hwaara, sspitzer, and bienvenu.)