Closed
Bug 132209
Opened 22 years ago
Closed 14 years ago
Many XUL documents are invalid XML (<!DOCTYPE window ...>)
Categories
(SeaMonkey :: UI Design, defect)
SeaMonkey
UI Design
Tracking
(Not tracked)
RESOLVED
EXPIRED
People
(Reporter: WeirdAl, Assigned: jag+mozilla)
Details
Attachments
(2 files, 1 obsolete file)
2.01 KB,
text/plain
|
Details | |
83.80 KB,
patch
|
Details | Diff | Splinter Review |
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.)
Reporter | ||
Comment 1•22 years ago
|
||
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 ...>)
Comment 2•22 years ago
|
||
-> 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
Updated•22 years ago
|
OS: Windows 98 → All
Hardware: PC → All
Reporter | ||
Comment 4•22 years ago
|
||
http://lxr.mozilla.org/seamonkey/search?string=%5E%5C%3C%5C%21DOCTYPE+window%3B This shows 392 files with "<!DOCTYPE window" at the beginning of a line. http://lxr.mozilla.org/seamonkey/search?string=%5E%5C%3Cwindow®exp=on This shows 187 files with "<window" at the beginning of a line. javascript:alert(document.getElementsByTagName("a").length - 4) is the line I used to determine this. 392 - 187 == 205. Two hundred five files with this bug in a verifiable manner. `8( Can someone write a better LXR regular expression to isolate those 205? Or should I attempt a JavaScript to do that for me with these two pages?
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.
Reporter | ||
Comment 6•22 years ago
|
||
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.
Comment 7•22 years ago
|
||
$ links -source "http://bugzilla.mozilla.org/attachment.cgi?id=92524&action=view" | grep -- '-<' | wc -l Shows 303 doctype changes.
Reporter | ||
Comment 8•22 years ago
|
||
Well, the tree's open... trudelle? Can we get a review here?
Keywords: review
Assignee | ||
Comment 9•22 years ago
|
||
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 10•22 years ago
|
||
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+
Comment 11•22 years ago
|
||
moa=kaie for changes in security/manager/
Comment 12•22 years ago
|
||
Changes below mailnews/extensions/smime fine with me, too.
Comment 13•22 years ago
|
||
Mikep: This bug also applies to some of the calendar XUL files, especially XUL overlays.
Comment 14•22 years ago
|
||
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.
Comment 15•22 years ago
|
||
moa=ccarlen for profile changes.
Comment 16•22 years ago
|
||
moa=jbetak for language/content pack changes
Comment 17•22 years ago
|
||
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.
Reporter | ||
Comment 18•22 years ago
|
||
Wow, what a firestorm! :) Who else do we need to bless this patch?
Comment 19•22 years ago
|
||
moa=dveditz for xpinstall change, although that file isn't included at the moment.
Reporter | ||
Comment 20•22 years ago
|
||
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
Comment 21•22 years ago
|
||
hm, Alex, I'm pretty sure that timeless has checked this partch in gradually, every time that there was an moa.
Comment 22•22 years ago
|
||
Mass moving all of my open Nav/toolkit bugs to new owner.
Assignee: trudelle → sgehani
Comment 23•21 years ago
|
||
assuming you've tested and nothing's broken, moa on the mailnews changes.
Status: NEW → ASSIGNED
Comment 24•21 years ago
|
||
Comment on attachment 92524 [details] [diff] [review] Patch making DOCTYPEs agree with root elements chatzilla and venkman changes are ok by me.
Comment 25•21 years ago
|
||
Comment on attachment 92524 [details] [diff] [review] Patch making DOCTYPEs agree with root elements inspector changes look fine to me
Comment 26•21 years ago
|
||
for editor files, moa=brade
Comment 27•21 years ago
|
||
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. =) ]
Comment 28•21 years ago
|
||
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
Comment 29•21 years ago
|
||
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.
Comment 30•21 years ago
|
||
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
Comment 32•21 years ago
|
||
r=varga on extensions/sql
Updated•20 years ago
|
Product: Core → Mozilla Application Suite
Updated•16 years ago
|
Assignee: samir_bugzilla → jag
Status: ASSIGNED → NEW
QA Contact: pawyskoczka
Target Milestone: mozilla1.2beta → ---
Comment 33•15 years ago
|
||
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
Comment 34•14 years ago
|
||
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
Closed: 14 years ago
Resolution: --- → EXPIRED
You need to log in
before you can comment on or make changes to this bug.
Description
•