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

RESOLVED FIXED in mozilla0.9.3

Status

()

RESOLVED FIXED
18 years ago
10 years ago

People

(Reporter: hyatt, Assigned: hamfastgamgee)

Tracking

Trunk
mozilla0.9.3
All
Windows 2000
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(19 attachments)

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
(Reporter)

Description

18 years ago
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.
(Reporter)

Updated

18 years ago
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
(Reporter)

Updated

18 years ago
Blocks: 70753
or, better yet, hbox & vbox.

Comment 3

18 years ago

*** This bug has been marked as a duplicate of 73671 ***
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → DUPLICATE
(Reporter)

Comment 4

18 years ago
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">
(Reporter)

Updated

18 years ago
Status: REOPENED → ASSIGNED
(Reporter)

Updated

18 years ago
Summary: [XUL Syntax] Replace <* align="horizontal|vertical"> with <box orient="horizontal|vertical"> → [XUL Syntax] Replace <boxDerivedTag align="horizontal|vertical"> with <boxDerivedTag orient="horizontal|vertical">
(Assignee)

Comment 5

18 years ago
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)? :)
(Assignee)

Comment 6

18 years ago
Created attachment 36389 [details] [diff] [review]
Patch to the files packaged in jars, sans Mac Classic (and maybe Win Classic, although it should have patched that, too)
(Assignee)

Comment 7

18 years ago
Created attachment 36390 [details] [diff] [review]
Patch to the files in the res directory (non-jarred)
(Assignee)

Comment 8

18 years ago
Created attachment 36674 [details] [diff] [review]
New patch, encompassing the entire tree (including Mac Classic)
(Assignee)

Comment 9

18 years ago
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).
(Assignee)

Comment 10

18 years ago
Created attachment 36768 [details] [diff] [review]
Patch for this bug, bug 70748, and bug 65428
(Assignee)

Comment 11

18 years ago
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. :)
(Assignee)

Comment 12

18 years ago
Created attachment 36769 [details] [diff] [review]
Correct patch (all problems stated solved)
(Assignee)

Comment 13

18 years ago
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
(Assignee)

Comment 14

18 years ago
Created attachment 37517 [details] [diff] [review]
Proposed fix for all mentioned changes and bugs
(Assignee)

Comment 15

18 years ago
Created attachment 38007 [details] [diff] [review]
Patch that should (knock on wood) be ready for r=/sr=
(Assignee)

Comment 16

18 years ago
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
(Assignee)

Comment 17

18 years ago
Created attachment 38718 [details] [diff] [review]
Patch with r=timeless, ready for sr=
(Assignee)

Comment 18

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

Comment 19

18 years ago
Will look into sr'ing tomorrow.
Whiteboard: [XUL1.0] → [XUL1.0][br]
(Assignee)

Comment 20

18 years ago
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.
(Assignee)

Comment 21

18 years ago
Created attachment 39635 [details] [diff] [review]
Hopefully, the last patch this bug'll need, diffed to the tip of 0.9.2
(Assignee)

Comment 22

18 years ago
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.
(Assignee)

Comment 23

18 years ago
Created attachment 40160 [details] [diff] [review]
Patch, updated to the trunk as of 2:30 PDT today
(Assignee)

Comment 24

18 years ago
Created attachment 40269 [details] [diff] [review]
Final patch, with the one bug fixed (ready for carpool)
(Assignee)

Comment 25

18 years ago
Created attachment 40457 [details] [diff] [review]
Ok.  Let's carpool this one, today.  Updated for the Modern changes from last night

Comment 26

18 years ago
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
(Assignee)

Comment 27

18 years ago
Created attachment 40617 [details] [diff] [review]
Patch that's absolutely ready for carpool (as said in an email last night)

Comment 28

18 years ago
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
(Assignee)

Comment 29

18 years ago
Created attachment 40848 [details] [diff] [review]
New patch for blake (using diff -u, but not -w)
(Assignee)

Comment 30

18 years ago
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).
(Assignee)

Comment 31

18 years ago
Created attachment 40875 [details] [diff] [review]
Refresh, because of Chatzilla and prefs dialog changes
(Assignee)

Comment 32

18 years ago
Created attachment 41364 [details] [diff] [review]
Smaller CSS/JS/XBL patch, to be checked in to allow XUL changes to go in easier

Comment 33

18 years ago
attachment 41364 [details] [diff] [review] checked in. no conflicts.
(Assignee)

Comment 34

18 years ago
Created attachment 41640 [details] [diff] [review]
XUL Changes, to land today
(Assignee)

Comment 35

18 years ago
Created attachment 41644 [details] [diff] [review]
Minor CSS changes (align="vertical" deprecation will be part of another bug)

Comment 36

18 years ago
checked in everything but security ...
(Assignee)

Comment 37

18 years ago
Created attachment 41649 [details] [diff] [review]
Security diff

Comment 38

18 years ago
Security changes checked in.
(Assignee)

Comment 39

18 years ago
Woohoo!
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago18 years ago
Resolution: --- → FIXED
(Assignee)

Comment 40

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

Comment 41

18 years ago
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.

Comment 44

18 years ago
Classic splitter CSS still uses window/box[align="vertical"] :-(
(Assignee)

Comment 45

18 years ago
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.)

Updated

10 years ago
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.