Closed Bug 132209 Opened 22 years ago Closed 14 years ago

Many XUL documents are invalid XML (<!DOCTYPE window ...>)

Categories

(SeaMonkey :: UI Design, defect)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED EXPIRED

People

(Reporter: WeirdAl, Assigned: jag+mozilla)

Details

Attachments

(2 files, 1 obsolete file)

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
->1.2
Target Milestone: --- → mozilla1.2alpha
OS: Windows 98 → All
Hardware: PC → All
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&regexp=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.
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?
Keywords: review
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
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. =) ]
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
Product: Core → Mozilla Application Suite
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
Closed: 14 years ago
Resolution: --- → EXPIRED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: