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

RESOLVED EXPIRED

Status

--
minor
RESOLVED EXPIRED
17 years ago
9 years ago

People

(Reporter: WeirdAl, Assigned: jag+mozilla)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

17 years ago
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

17 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

17 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

Comment 3

17 years ago
->1.2
Target Milestone: --- → mozilla1.2alpha

Updated

17 years ago
OS: Windows 98 → All
Hardware: PC → All
(Reporter)

Comment 4

17 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&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?

Comment 5

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

Comment 6

17 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.
$ links -source
"http://bugzilla.mozilla.org/attachment.cgi?id=92524&action=view" | grep -- 
'-<' | wc -l
Shows 303 doctype changes.
(Reporter)

Comment 8

17 years ago
Well, the tree's open... trudelle?  Can we get a review here?
Keywords: review
(Assignee)

Comment 9

17 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

17 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+
moa=kaie for changes in security/manager/
Changes below mailnews/extensions/smime fine with me, too.

Comment 13

17 years ago
Mikep: This bug also applies to some of the calendar XUL files, especially XUL
overlays.

Comment 14

17 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.
moa=ccarlen for profile changes.
moa=jbetak for language/content pack changes

Comment 17

17 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

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

Comment 20

17 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
hm, Alex, I'm pretty sure that timeless has checked this partch in gradually,
every time that there was an moa.

Comment 22

16 years ago
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 24

16 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 on attachment 92524 [details] [diff] [review]
Patch making DOCTYPEs agree with root elements

inspector changes look fine to me

Comment 26

16 years ago
for editor files, moa=brade

Comment 27

16 years ago
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. =) ]

Comment 28

16 years ago
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

Comment 29

16 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

16 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

16 years ago
r=varga on extensions/sql
Product: Core → Mozilla Application Suite
Assignee: samir_bugzilla → jag
Status: ASSIGNED → NEW
QA Contact: pawyskoczka
Target Milestone: mozilla1.2beta → ---

Comment 33

10 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

9 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
Last Resolved: 9 years ago
Resolution: --- → EXPIRED
You need to log in before you can comment on or make changes to this bug.