Ladies and gentlemen, I've been looking into XUL a little bit lately, and there's something that bothers me about a few XUL documents I've seen, and about the current XUL documentation. There appear to be several XUL documents with a <!DOCTYPE window ... > tag, and a root element which is not <window>...</window>. Such documents are not valid XML documents. I know this doesn't matter much for Mozilla 1.0, as Mozilla is not a validating XML parser. But if the Mozilla suite ever does turn on validation of XML documents, I shudder to think of the fallout from invalid XUL upon the suite. To my knowledge, the documentation on XUL, even at XULPlanet, backs up this mistake. Opinions? (reposted from XPFE newsgroup, dated 02-Feb-02. Bug filed at request of heikki. I've been offline a long time.)
cc'ing heikki, as he is the one who asked me to file this.
Summary: Many XUL documents are invalid XML (<!DOCTYPE window ...> → Many XUL documents are invalid XML (<!DOCTYPE window ...>)
-> XP Apps [front end] (although this is really all front-end xul files, including mailnews, extensions, etc.)
Assignee: hyatt → trudelle
Component: XP Toolkit/Widgets: XUL → XP Apps
QA Contact: shrir → paw
Target Milestone: --- → mozilla1.2alpha
Created attachment 92524 [details] [diff] [review] Patch making DOCTYPEs agree with root elements For every XUL file that had both an explicit DOCTYPE and a root element (and for which the actual root element was not what the DOCTYPE specified), the DOCTYPE declaration has been changed to match the root element.
Riceman Tim, you rock. How many of these did you fix? :-) I know we're in lockdown for 1.1, and this probably is not important enough to land for 1.1, but I'm wondering if we can start seeking r=/sr= on this.
$ links -source "http://bugzilla.mozilla.org/attachment.cgi?id=92524&action=view" | grep -- '-<' | wc -l Shows 303 doctype changes.
Well, the tree's open... trudelle? Can we get a review here?
Comment on attachment 92524 [details] [diff] [review] Patch making DOCTYPEs agree with root elements rs=jag and module owner approval. (Rubberstamp meaning I agree with the change, but haven't checked every change to make sure the doctypes now match, something the reviewer can do). It would help the reviewer if you did your patch with -u10 (10 lines of context) so he/she can see the matching root element in the patch instead of having to look it up.
Attachment #92524 - Flags: superreview+
Comment on attachment 92524 [details] [diff] [review] Patch making DOCTYPEs agree with root elements a bunch of files weren't available to patch, that's not a concern. the only file i'm not sure about is mozilla/xpfe/browser/samples/dexparamdialog.xul
Attachment #92524 - Flags: review+
moa=kaie for changes in security/manager/
Changes below mailnews/extensions/smime fine with me, too.
Mikep: This bug also applies to some of the calendar XUL files, especially XUL overlays.
The dexparams delta is a glitch in my detection algorithm. I used something similar to /<(\w+)/ to find the name of the root element, which led to that erroneous change.
moa=ccarlen for profile changes.
moa=jbetak for language/content pack changes
Bug 166373 is assigned to me and deals with the calendar files. I will fix those. Since calendar isn't yet built by default, I don't see a reason to mark a dependancy on this bug.
Wow, what a firestorm! :) Who else do we need to bless this patch?
moa=dveditz for xpinstall change, although that file isn't included at the moment.
This patch is almost two months old, has gotten r= and sr=, five moa= rubberstamps, and involves code that almost certainly would not affect the operation of the Mozilla suite. If we don't check it in soon (and imho there's no reason not to), we'll be facing bitrot and have to build a new patch. Moving milestone to 1.2 beta as 1.2 alpha has already shipped.
Target Milestone: mozilla1.2alpha → mozilla1.2beta
hm, Alex, I'm pretty sure that timeless has checked this partch in gradually, every time that there was an moa.
Mass moving all of my open Nav/toolkit bugs to new owner.
Assignee: trudelle → sgehani
assuming you've tested and nothing's broken, moa on the mailnews changes.
Status: NEW → ASSIGNED
Comment on attachment 92524 [details] [diff] [review] Patch making DOCTYPEs agree with root elements chatzilla and venkman changes are ok by me.
Comment on attachment 92524 [details] [diff] [review] Patch making DOCTYPEs agree with root elements inspector changes look fine to me
for editor files, moa=brade
Created attachment 126891 [details] Perl script to detect these errors Just so anyone can find these errors easily, here is a perl script to detect the errors. A lot of my old patch bitrotted, so I'm using that script to concoct a new patch (coming up). [Yes, it could be faster by not parsing past the first element, or by using a regular expression, but I'm too lazy and fast computers are cheap. =) ]
Created attachment 126892 [details] [diff] [review] Un-bitrotted patch A lot of changes in the old patch have been fixed already. This patch includes enough context so that both the (changed) doctype and the root element are visible for each file. There are 53 files affected by this patch, as follows: 3 editor 1 extensions/cview 2 cascades 1 extensions/help 10 chatzilla 3 extensions/sql 1 extensions/transformiix 19 extensions/wallet 1 extensions/xmlterm 6 mailnews 2 mailnews/extensions/mailviews 2 xpfe/communicator 1 xpfe/components/prefwindow There are errors in toolkit and browser (firebird's neck of the woods), but those files are not valid XML in the source tree (they use a preprocessor on the source to produce valid XML output). I have not done any other directories outside of the default tree, but I can if there is a request for me to.
Attachment #92524 - Attachment is obsolete: true
Oops, meant to comment while I CCed. If I CCed you just now, I think I need moa from you for changes in your realm.
technically i'm not moa for wallet, but this is simple enough not to bug dveditz. moa=dwitte.
same here, i'm not owner of transformiix, but the change is simple enough. r=sicking on the transformiix change
r=varga on extensions/sql
Assignee: samir_bugzilla → jag
Status: ASSIGNED → NEW
QA Contact: pawyskoczka
Target Milestone: mozilla1.2beta → ---
MASS-CHANGE: This bug report is registered in the SeaMonkey product, but has been without a comment since the inception of the SeaMonkey project. This means that it was logged against the old Mozilla suite and we cannot determine that it's still valid for the current SeaMonkey suite. Because of this, we are setting it to an UNCONFIRMED state. If you can confirm that this report still applies to current SeaMonkey 2.x nightly builds, please set it back to the NEW state along with a comment on how you reproduced it on what Build ID, or if it's an enhancement request, why it's still worth implementing and in what way. If you can confirm that the report doesn't apply to current SeaMonkey 2.x nightly builds, please set it to the appropriate RESOLVED state (WORKSFORME, INVALID, WONTFIX, or similar). If no action happens within the next few months, we move this bug report to an EXPIRED state. Query tag for this change: mass-UNCONFIRM-20090614
Status: NEW → UNCONFIRMED
MASS-CHANGE: This bug report is registered in the SeaMonkey product, but still has no comment since the inception of the SeaMonkey project 5 years ago. Because of this, we're resolving the bug as EXPIRED. If you still can reproduce the bug on SeaMonkey 2 or otherwise think it's still valid, please REOPEN it and if it is a platform or toolkit issue, move it to the according component. Query tag for this change: EXPIRED-20100420
Status: UNCONFIRMED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → EXPIRED
You need to log in before you can comment on or make changes to this bug.