Closed Bug 70857 Opened 21 years ago Closed 21 years ago

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

Categories

(Core :: XUL, defect)

All
Windows 2000
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla0.9.3

People

(Reporter: hyatt, Assigned: hamfastgamgee)

References

Details

(Whiteboard: [XUL1.0][br])

Attachments

(19 files)

60.53 KB, patch
Details | Diff | Splinter Review
16.46 KB, patch
Details | Diff | Splinter Review
133.02 KB, patch
Details | Diff | Splinter Review
405.92 KB, patch
Details | Diff | Splinter Review
396.78 KB, patch
Details | Diff | Splinter Review
760.13 KB, patch
Details | Diff | Splinter Review
745.86 KB, patch
Details | Diff | Splinter Review
733.85 KB, patch
Details | Diff | Splinter Review
740.34 KB, patch
Details | Diff | Splinter Review
744.11 KB, patch
Details | Diff | Splinter Review
744.73 KB, patch
Details | Diff | Splinter Review
747.62 KB, patch
Details | Diff | Splinter Review
744.42 KB, patch
Details | Diff | Splinter Review
742.77 KB, patch
Details | Diff | Splinter Review
742.77 KB, patch
Details | Diff | Splinter Review
16.30 KB, patch
Details | Diff | Splinter Review
722.37 KB, patch
Details | Diff | Splinter Review
1.97 KB, patch
Details | Diff | Splinter Review
39.94 KB, patch
Details | Diff | Splinter Review
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.
Status: NEW → ASSIGNED
Summary: Replace <box align="horizontal|vertical"> with <box orient="horizontal|vertical"> → [XUL Syntax] Replace <box align="horizontal|vertical"> with <box orient="horizontal|vertical">
Whiteboard: [XUL1.0]
Target Milestone: --- → mozilla1.0
Blocks: 70753
or, better yet, hbox & vbox.

*** This bug has been marked as a duplicate of 73671 ***
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → DUPLICATE
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.

Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Summary: [XUL Syntax] Replace <box align="horizontal|vertical"> with <box orient="horizontal|vertical"> → [XUL Syntax] Replace <* align="horizontal|vertical"> with <box orient="horizontal|vertical">
Status: REOPENED → ASSIGNED
Summary: [XUL Syntax] Replace <* align="horizontal|vertical"> with <box orient="horizontal|vertical"> → [XUL Syntax] Replace <boxDerivedTag align="horizontal|vertical"> with <boxDerivedTag orient="horizontal|vertical">
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)? :)
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).
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. :)
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.
Assignee: hyatt → andersma
Status: ASSIGNED → NEW
Target Milestone: mozilla1.0 → mozilla0.9.3
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.
Status: NEW → ASSIGNED
This latest patch has r=timeless (given on IRC) and is awaiting sr=blake. :)
Will look into sr'ing tomorrow.
Whiteboard: [XUL1.0] → [XUL1.0][br]
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.
Ok.  Let's get this landed, shall we?  Cc:ing jj@netscape.com 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.
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
Hardware: PC → All
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
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).
attachment 41364 [details] [diff] [review] checked in. no conflicts.
checked in everything but security ...
Attached patch Security diffSplinter Review
Security changes checked in.
Woohoo!
Status: ASSIGNED → RESOLVED
Closed: 21 years ago21 years ago
Resolution: --- → FIXED
See also bug 89999 for final changes, once commercial is handled.
this fix seems to have caused bug 90055, if anyone involved in this bug wants to
look at that, that would be nice.
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?


bug #90068 tracks the mailnews horkage.

the rest of the product may be horked as well.
Classic splitter CSS still uses window/box[align="vertical"] :-(
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.)
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: jrgmorrison → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.