Closed Bug 111514 Opened 23 years ago Closed 21 years ago

document.body has no properties in XHTML as application/xhtml+xml

Categories

(Core :: DOM: Core & HTML, defect, P2)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: WeirdAl, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: testcase, xhtml)

Attachments

(2 files, 3 obsolete files)

Testcase using document.body
519 bytes, application/xhtml+xml
Details
Merge the above changes to the trunk, and share ID matching code between nsHTMLDocument and nsXMLDocument.
44.33 KB, patch
sicking
: review+
Details | Diff | Splinter Review 
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.6+) Gecko/20011122
BuildID:    Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.6+) Gecko/20011122

In an XHTML document saved as .xml locally, I receive an error "document.body
has no properties".  This means I would have to use
document.getElementsByTagName("body")[0] instead.

Reproducible: Always
Steps to Reproduce:
1.  Load attached testcase.

Actual Results:  Two paragraphs; second one says "Hello World".

Expected Results:  One paragraph.

    
  
Keywords: testcase, 
          
            xhtml

    
    

    
  
This seems like a DOM0 feature that we would do well to not support in XHTML as
XML....

    
    

    
  
I beg to differ.  This is a DOM-1 HTML issue.

http://www.w3.org/TR/1998/REC-DOM-Level-1-19981001/level-one-html.html

HTMLDocument has body as a property, referring to HTMLBodyElement. 
HTMLBodyElement has six properties in DOM-1: aLink, background, bgColor, link,
text, and vLink.



    
  
Depends on: 111536

    
    

    
  
Yes, but this is an xml document, so we don't _have_ an HTMLDocument. If this
doesn't work when the mimetype is application/xhtml+xml, then thats a bug, but I
don't think the iface should be present for text/xml documents.

hixie: ?

    
    

    
  
See bug 111536 for an application/xhtml+xml testcase.

    
    

    
  
I agree that we should create HTML document objects for application/xhtml+xml
documents, but there's not really a standard that say we should do that...

    
    

    
  
*** Bug 109373 has been marked as a duplicate of this bug. ***
No longer depends on: 111536

    
    

    
  
*** Bug 111536 has been marked as a duplicate of this bug. ***

    
    

    
  
Accepting...
Status: NEW → ASSIGNED
OS: Windows 98 → All
Priority: -- → P2
Hardware: PC → All
Target Milestone: --- → mozilla0.9.8

    
    

    
  
jst: The dom-2-core spec says: "The interfaces in this specification are
designed for HTML 4.0 documents, as well as for XHTML 1.0 documents.", so it
sort of implies it.

Whether we should do so for text/xml with an xhtml doctype is another issue...

    
    

    
  
These DOM features should be available for text/xml documents.  document.links 
is much cleaner than document.getElementsByTagName('a') followed by a check to 
make sure each <a> is a link rather than an anchor.  document.links is also 
more likely to work on documents that use a method other than <a> to create 
links.

See http://www.squarefree.com/bookmarklets/ for some examples of why it's 
useful to be able to run your javascript code on someone else's page.

    
    

    
  
Not for all text/xml.  For XHTML documents in particular -- say, 

if (document.documentElement.namespaceURI == "http://www.w3.org/1999/xhtml") {

or if it has one of the three XHTML <!DOCTYPE > tags.

    
    

    
  
According to the [X]HTML WG user agents should *not* sniff the content of the
document to figure out what type of document should be created, i.e. if the mime
type is application/xhtml+xml, it's a XHTML document, if it's text/xml, it's
not, it's just a XML document that happens to use elements in the XHTML
namespace, nothing more.

    
    

    
  
Let's just wait for the W3C EDOM work to give us the answers. :-)

    
    

    
  
I don't think EDOM will answer this question. EDOM might give us some new
functionality that allows script writers to treat a XML document as a HTML
document, but it won't say wether or not we should do namespace sniffing on
content before deciding what type of document to initially create.

    
    

    
  
For the record, if you look at the application/xhtml+xml testcase in 111536
(http://bugzilla.mozilla.org/attachment.cgi?id=58928&action=view ), you'll see
there's quite a few undefined properties besides the collections I mentioned. 
It may be appropriate to resummarize the bug appropriately.
Target Milestone: mozilla0.9.8 → mozilla0.9.9

    
    

    
  
this happens for document.applets, document.image, document.links, document.forms,
document.anchors as well

    
    

    
  
JST: re: comment #13, The problem with not building these properties for
text/xml documents is that if you serve XHTML documents as text/html, you're not
able to style non-XHTML elements in the same document with CSS. The workaround
for that is to serve these documents as text/xml.

As far as I understand it, the offshoot of not setting these properties for
text/xml documents is things like the Password Manager, Page Info, etc, break.
See the bug I just reported (bug 125166) for an example.


    
    

    
  
If you want an XHTML document, then you should serve your data using the XHTML
mimetype (application/xhtml+xml). There are bugs on that not doing the right
thing in mozilla, but I believe that will be fixed for mozilla1.0, where as this
will not be fixed for mozilla1.0, if ever.

    
    

    
  
Pushing out to mozilla1.1.
Target Milestone: mozilla0.9.9 → mozilla1.1

    
    

    
  
*** Bug 126177 has been marked as a duplicate of this bug. ***

    
    

    
  
*** Bug 126288 has been marked as a duplicate of this bug. ***

    
    

    
  
Regarding Comment 20: I've been looking for an equivalent bug on
application/xhtml+xml that's targeted for Mozilla 1.0, but I just don't see one.

If this bug is only about pages served as text/xml, then shouldn't bug 126177
and bug 126288, which focus on application/xhtml+xml, be tied to another bug? 
Especially if application/xhtml+xml has a higher priority?  (I'd like to be able
to follow the progress on that.)

    
    

    
  
I personally believe that the class of DOM document object generated should
depend on the namespace of the root element. That isn't sniffing, since it is
very well defined and logical. It also would mean we could finally get rid of
the hacky XUL mime type issue.

    
    

    
  
Hixie, there's obviously a lot of work involved in that, so I'm assuming such a
change wouldn't get implemented until post 1.0?

+1 for the idea anyway..

    
    

    
  
I doubt this would be implemented in the next 5 weeks, right, but then this bug
isn't targetted for 1.0 anyway.

    
    

    
  
*** Bug 68195 has been marked as a duplicate of this bug. ***

    
    

    
  
Incidentally, to quote from IRC:

<Hixie> what i really think should happen is that the DOM's document object
shouldn't be for a single namespace
<Hixie> there should be one DOM Core document, and it should be castable to
other types
<Hixie> like with the Element DOM node
<Hixie> you can't use DOCTYPEs and MIME types because they don't cope with
mixing namespaces
<Hixie> imagine a document with mixed XHTNL, MathML, SVG and XUL. All four
namespaces need a way to access their utility and factory methods. What the root
node happens to be shouldn't suddenly make all scripts break.
<Hixie> many elements can be cast to StyleElement
<Hixie> and all of them can be cast to XBLElement and UIElement (in my world)
<Hixie> all documents should be DOM core documents that are castable to all
other document types so that all scripts will work regardless of what the root
element is, what mime type exists and what doctype there is
<Hixie> a requirement of whatever solution we pick, imho, is that any XHTML
element with an onclick handler should be clonable and insertable into _any_
document and the script should still work.

    
    

    
  
Hixie, if all documents were castable to all the kind of documents, it would
mean we would have to implement a nsMonstruousDocument class which implements
ALL the interfaces of ALL the kind of documents. I don't dare thinking of the
footprint and perf costs of such a monster. Also, how do you determine, from js,
which method to call? For example, document.write() is supposed to work in HTML,
but not in any other document type. However it will be implemented by
nsMonstruousDocument. Since in JS there is interface flattening (no explicit
casting is needed), document.write() will be available to all kind of documents.
Do you have a solution?

I have no opinion however on whether we should sniff, doctype, root element
namespace, or whatever. Do as you like :-)

    
    

    
  
Fabian: Ok, so tell me this: How do I, in the midst of an SVG document, in a
little <svg:foreignObject> embedded XHTML form's script, use the document.forms
API? Or, how do I use the XUL Document's addBroadcastListenerFor() method from
script associated with XUL elements embedded in that XHTML fragment?

Those aren't just academic questions. We _must_ provide solutions for that. The
first comment in this bug is just the tip of the iceberg.

Regarding interface flatening and JS: it's actually not a big deal. There are
very few name clashes, and those could be resolved simply by saying that the one
appropriate to the root element's namespace wins. (e.g. document.title would
look for an <html:title> in an XHTML doc and <svg:title> in an SVG one,
regardless of what other elements were embedded in it.)

And if that isn't enough -- well, that's JS' problem. Maybe JS shouldn't be so
eager to flatten everything. ;-) We could provide some mechanism to get to a
specific interface from the main document object, or something.

    
    

    
  
Yeesh.  I look at this whole mess and think I'd better make the point jst made 
to me:  that this bug should apply only to application/xhtml+xml.

Otherwise, this can of worms is going to turn out to be a truck full of them.

Although I filed this bug and I have some discretion to change the summary as I 
see fit, I'd recommend jst as the bug owner change the summary to 
read "document.body has no properties in XHTML as application/xhtml+xml"

On a personal note, I like Hixie's suggestion...

    
    

    
  
Do you think there is enough time to complete this patch? I think at the very
least we should checkin sicking's patch in bug 130000. This could make adoption
of the correct xhtml mime-type painful in the future.
Note on the status of the patch: in my tree nsXHTMLDocument is done and the
patch in bug 130000 has been integrated, but I still need to modify the document
loading methods and do the testing.

    
    

    
  
yeah, heikki's right. It wouldn't help anything and it's the wrong way to do 
things. (what a shame with such a cool bug# too ;-) )

    
    

    
  
fabian: there's one spot i missed in bug 130000; make sure that ::GetCSSLoader 
returns a case-sensitive non-quirky nsICSSLoader

    
    

    
  
*** Bug 134898 has been marked as a duplicate of this bug. ***

    
    

    
  
*** Bug 145471 has been marked as a duplicate of this bug. ***

    
    

    
  
*** Bug 149916 has been marked as a duplicate of this bug. ***

    
    

    
  
*** Bug 163974 has been marked as a duplicate of this bug. ***

    
    

    
  
jst, care to retarget?  1.1 alpha is long gone.

    
    

    
  
*** Bug 176115 has been marked as a duplicate of this bug. ***

    
    

    
  
So, it seems like this so called standards compliant browser will never ever
have full support for XHTML and DOM. I guess it is more important making new
corn flower blue icons for IRC or something than having a functional XHTML browser.

But exactly how do I circumvent this bug to accomplish something like
document.write with the DOM core?

    
    

    
  
use document.createElement and friends

    
    

    
  
"use document.createElement and friends"

Unfortunately, those don't work as expected (see bug 126288, "XHTML elements
added through DOM are ignored.")

And that bug's been marked as a duplicate of this one.

    
    

    
  
well, the "and friends" bit is important. In particular you want createElementNS
for creating elements.

    
    

    
  
Well, that works. Thank you. Now then... how do I implement a core version of
document.cookie?

    
    

    
  
Since 1.1a is out for loong time now, the target milestone is missed... :)

    
    

    
  
*** Bug 178399 has been marked as a duplicate of this bug. ***
Target Milestone: mozilla1.1alpha → ---

    
    

    
  
Is this why the CSS rule

body { background: #ccccff; }

works when an XHTML document is served as text/html but not when served as
application/xhtml+xml ?


    
    

    
  
It works in both cases.  See http://www.w3.org/TR/CSS21/colors.html#q2,
paragraph 4.  That behavior, unlike this, is not a bug and will not change.

    
    

    
  
*** Bug 185189 has been marked as a duplicate of this bug. ***

    
    

    
  
*** Bug 186298 has been marked as a duplicate of this bug. ***

    
    

    
  
Two issues:
1) Should an XMLDocument have properties of HTMLDocument?

2) Should a file served as application/xhtml+xml use HTMLDocument or XMLDocument?

I address issue 1, Should an XMLDocument have properties of HTMLDocument?
No. This bug should be marked INVALID. XMLDocument does not have a body property.

a) An XMLDocument is not an HTMLDocument
b) An XMLDocument does not have a property named body.

Is the attachment an XMLDocument? 
Yes and it is also an HTMLDocument. Anytime you see an "is a" relationship,
consider inheritance. It is an XMLDocument, but more specifically, it is an
HTMLDocument. HTMLDocument inherits from XMLDocument. The HTMLDocument is the
objects true type, but the object is instantiated as an XMLDocument. Since
XMLDocuments do not have a property named body, calling document.body should be
handled by the language as an undefined property (an error in Java, and
"undefined" value in JavaScript).

attachment 58900 [details] uses mime-type of text/xml. Bugzilla does not support
attachments using the mime-type "application/xhtml+xml," (see bug 111520), so my
attachment is hosted on my own space.

My attachment sends a content-type header of application/xhtml+xml. The 
<% response.setContentType("application/xhtml+xml") %>

Please see: http://dhtmlkitchen.com/java/xht.jsp

An document served as text/xml should not be loaded as an HTMLDocument; it
should be loaded as an xml document. 


Hixie, I think this is way off:
"...
Regarding interface flatening and JS: it's actually not a big deal. There are
very few name clashes, and those could be resolved simply by saying that the one
appropriate to the root element's namespace wins. (e.g. document.title would
look for an <html:title> in an XHTML doc and <svg:title> in an SVG one,
regardless of what other elements were embedded in it.)
..."
The DOM Core has the answer to this problem. The answer is
document.getElementsByTagName. What if there is no document title? Or what if
there are two or four or eleven titles, each being in a different NS? Again Dom
Core answers with Document instance method getElementsByTagNameNS. 

A monolithic wrapper class does not exist to achieve this result, but could
certainly be created in ECMAScript DOM language bindings. 


Hixie:
"...
how do I use the XUL Document's addBroadcastListenerFor() method from
script associated with XUL elements embedded in that XHTML fragment?
..."

That is a question to be answered by EDOM.
http://www.w3.org/TR/2001/WD-DOM-Requirements-20010419/#Level-3-Embedded

    
    

    
  
> (see bug 111520)

Which is not relevant; that's why we provide a "Other" field that lets you type
the darn MIME type.

    
    

    
  
Uh, Mr. Smith, may I point you to bug 111536?  Particularly attachment 58928 [details] ...
Comment 5 and comment 16.

I'd agree with you as to point 1 you make.  It's issue 2 that's causing this 
whole debate.
Summary: document.body has no properties in XHTML as XML → document.body has no properties in XHTML as application/xhtml+xml

    
    

    
  
Comment on attachment 58900 [details]
Testcase using document.body

Changing the mime-type of the attachment to reflect the real issue.

        Attachment #58900 -
        Attachment mime type: text/xml → application/xhtml+xml

    
    

    
  
"...The 'application/xhtml+xml' media type [RFC3236]
is the media type for XHTML Family document types..."

"...'application/xhtml+xml' SHOULD be used for serving
XHTML documents to XHTML user agents..."
- http://www.w3.org/TR/xhtml-media-types/#application-xhtml-xml


"...Developers need to take two things into account when writing code that works
on both HTML and XHTML documents. When comparing element or attribute names to
strings, the string compare needs to be case insensitive, or the element or
attribute name needs to be converted into lowercase before comparing against a
lowercase string. Second, when calling methods that are case insensitive when
used on a HTML document (such as getElementsByTagName() and namedItem()), the
string that is passed in should be lowercase..."
- http://www.w3.org/TR/DOM-Level-2-HTML/html.html#ID-5353782642

"...XHTML documents served as Internet media types text/xml, application/xml, or
application/xhtml+xml can also use the XML DOM..."

- http://www.w3.org/TR/xhtml1/#C_11

* Developers should serve up xhtml documents with application/xhtml+xml
content-type.
* Developer should take 2 things into account.

The document served up with application/xhtml+xml must be an html document. It
is not illegal to use the XML DOM, but using XML DOM is less desirable from a
developers standpoint. 


 As an explanation of the practical implications of using XMLDocument for
documents served with application/xhtml+xml, I offer the following story:

I follow the recommendation and use content negotiation to serve my documents 
with application/xhtml+xml, byt my DHTML won't work. I figure out that I have an
XMLDocument. document.body is easy to get around, but an alternative for
document.write is not practical when considering other browsers. 

My javascript is not dependent upon the accept-encoding; it is run in any
encoding. Coding for the least common denominator (XML DOM) might not work in
all the browsers I support and will also require more code, more testing, and
more maintenance. Since document.write does work in all the browsers I support,
I decide to use it an serve all my documents with content-type of text/html. I
know I'm not following the w3c recommendation, but I have to use what works.

This is a true story.



    
    

    
  
Mass-reassigning bugs.
Assignee: jst → dom_bugs
Status: ASSIGNED → NEW

    
    

    
  
So, if I serve a document as "application/xhtml+xml", it SHOULD be possible to
have it governed by the HTMLDocument DOM? 

Mozilla 1.4a doesn't do that, so document.cookie is inaccesible.

I don't care about "text/xml". For "application/xhtml+xml" this certainly is a bug.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Proposed fix (obsolete)
        — 
          — Splinter Review
      

    


  
This patch makes Mozilla create a HTML document (but a case sensitive one) for
XHTML documents served up as application/xhtml+xml. This also moves some code
that deals with the XML declaration from ns[I]XMLDocument to ns[I]Document.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Even better (obsolete)
        — 
          — Splinter Review
      

    


  
This makes the XPath evaluator work correctly (and be independent of HTML) in
this new world too.

        Attachment #118609 -
        Attachment is obsolete: true

        Attachment #118610 -
        Flags: superreview?(peterv)

        Attachment #118610 -
        Flags: review?(bugmail)
Blocks: 199399
Status: NEW → ASSIGNED

    
    

    
  
Comment on attachment 118610 [details] [diff] [review]
Even better

yumyum.
Just wanted to add that I personally like the changes to xpath.

    
    

    
  
Comment on attachment 118610 [details] [diff] [review]
Even better

I found some problems in this patch, new patch coming up later...

        Attachment #118610 -
        Attachment is obsolete: true

        Attachment #118610 -
        Flags: superreview?(peterv)

        Attachment #118610 -
        Flags: review?(bugmail)

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Better yet... (obsolete)
        — 
          — Splinter Review
      

    


  
All known problems fixed.

        Attachment #119147 -
        Flags: superreview?(heikki)

        Attachment #119147 -
        Flags: review?(bugmail)

    
    

    
  
 nsHTMLDocument::LookupNamespacePrefix(const nsAString& aNamespaceURI,
                                       nsAString& aPrefix)
 {
-  aPrefix.Truncate();
-  return NS_OK;
+  return nsDocument::LookupNamespacePrefix(aNamespaceURI, aPrefix);
 }

Why not just leave the function unimplemented?  That should make the parent
field it automatically, no?

+PRInt32
+GetDocumentNamespace(nsIContent *aContent)

What if aContent is not in a document?

    
    

    
  
nsHTMLDocument::LookupNamespacePrefix() is part of nsIDOMDocument which is
inherited by nsIDOMHTMLDocument, and thus must be implemented by nsHTMLDocument,
no way around that.

>+PRInt32
>+GetDocumentNamespace(nsIContent *aContent)
>
>What if aContent is not in a document?

That shouldn't be possible, this function is only called from functions that
iterate over content in a document. I can assert, or even assert and check if
that makes people feel better, but it really shouldn't happen.

    
    

    
  
> this function is only called from functions that iterate over content in a
> document

Ah, ok.  Fair enough.  Document this in assertion form, maybe?

    
    

    
  
Done.

    
    

    
  
This seems to ignore comment 25. Why do we want to decide what the document
object's interface is based on the MIME type? How is one supposed to access XUL
utility methods from an XHTML document, and vice versa?

    
    

    
  
Comment 25 is all very well and good, but the document I'm looking at has a MIME
type application/xhtml+xml and the correct root namespace:

   <html xmlns="http://www.w3.org/1999/xhtml">

and Mozilla still treats it as XMLDocument, not HTMLDocument. 

It seems to me that a MIME type of application/xhtml+xml (as opposed to
text/xml) is unambiguously an XHTML document, which should be governed by the
HTMLDocument DOM. 

If authors say contradictory things in their MIME-type and root namespace
declarations, then you somehow have to arbitrate between those contradictory
assertions.

If the MIME-type is text/xml, that's sufficiently uniformative that you HAVE to
look at the root namespace declaration to figure out what kind of document you
are actually looking at.

But I don't think that is situation being discussed here. What we have here is
unambiguously an XHTML document and unambiguously a Mozilla bug.

    
    

    
  
Glad this underway.

Ian wants a way to use methods for one type of XMLDocument (XUL) inside an XHTML
doc. Maybe some sort of narrow method, since casting is not available in js.

this is beyond scope of this bug, IMO.

A document with text/xml will be an XMLDocument, but maybe it's also really an
HTMLDocument under an XHTML doctype. In that case, the document's true type
would be HTMLDocument, requiring some type of narrowing to get that runtime
type. How should the loaded document implement the different interfaces? XHTML
document is loaded with XUL embedded, so now what can Ian do to get the methods
of the interface he wants? I'd suggest node.ownerDocument as a solution, but I
don't think that will work right now. 

Namespace sniffing on content would be problematic because you'd have to parse
the whole document first and this might bring up complications with scripts
using document.write. 

Ian, you should explain your solution this to jst if you can. I don't know c++
and I haven't read the source code, so this is beyond me.

jst, thank you for working on this.

    
    

    
  
Yup, any issues other than making Mozilla create a document object that
implements the HTMLDocument interface for XHTML documents loaded as
application/xhtml+xml is outside the scope of this bug, for such issues, please
file separate bugs.

    
    

    
  
If it is out of scope of this bug, then I think this bug should be WONTFIX.

The two solutions aren't really independent. An XHTML document doesn't stop
being an XHTML document when you change its MIME type from one */*+xml type to
another.

    
    

    
  
If Johnny Stenback's patch is implemented, XHTML documents sent with MIME-type
application/xhtml+xml will be governed by the HTMLDocument DOM. Those sent with
MIME-type text/xml will (still) be governed by the XMLDocument DOM.

Why can't Ian choose his MIME-type based on which DOM he wants to apply?

There is clearly a big can of worms with text/xml (which could be just about
anything). I don't pretend to have a general solution to that.

But I don't understand Ian's insistence that XHTML documents served up as
application/xhtml+xml should NOT be governed by the HTMLDocument DOM ("this bug
should be WONTFIX").

    
    

    
  
Fixing this bug would make it possible for people to serve up XHTML documents as
application/xhtml+xml and get a document object that implements the HTMLDocument
interface, why would we choose to not do that? Maybe this isn't the complete fix
for this problem, but it's the one we can do at this point in time, any kind of
document sniffing (which includes even looking at the root elements namespace)
is beyond our reach at this point, maybe fore 1.5 or beyond, but it won't happen
before that. Fixing this bug doesn't make the "real" fix any harder, or any
different, so Hixie, do you still feel that this should be WONTFIX (in case you
didn't realize, I strongly disagree with that)?

    
    

    
  
Cool.

    
    

    
  
jst: I don't mind the patch going in as a temporary placeholder, but it
shouldn't cause this bug to be marked RESOLVED FIXED. The original bug here was
"document.body has no properties in XHTML as XML". While it was morphed into a
bug specific to application/xhtml+xml, the original problem would still exist.


    
    

    
  
Ok, I didn't realize this had morphed, and I'm happy to morph it back when this
lands and leaving it open for further work.

    
    

    
  
Re comment 75:  text/xml and application/xml.  I really regret filing this bug
so generically to begin with...

Re the bug morphing:  blame me for that one.  I did that because I agree with
jst and Hixie's opinions for text/xml and app/xml.  I think once we check in a
patch for app/xhtml+xml, all interested parties should take a minute to reread
the entire bug report before commenting further.  It's getting out of hand, and
I'd be happy to open a thread on my weblog
(http://www.mozillazine.org/weblogs/weirdal ) for discussion.  Given the number
of cc's on this bug, and the resulting drain on the Bugzilla server, I just
don't know what to say anymore in here that would help.

        Attachment #119147 -
        Attachment is obsolete: true

        Attachment #119147 -
        Flags: superreview?(heikki)

        Attachment #119147 -
        Flags: review?(bugmail)

        Attachment #119355 -
        Flags: superreview?(peterv)

        Attachment #119355 -
        Flags: review?(bugmail)

    
    

    
  
Hmmm. Did this ever get checked in?

    
    

    
  
It needs reviews before it can be checked in.  See
http://mozilla.org/hacking/life-cycle.html

    
    

    
  
Sicking still says he'll review this, but he's traveling at the moment, and
won't be able to have a look until next week at the earliest. If someone else
wants to have a look, be my gues.

    
    

    
  
Comment on attachment 119355 [details] [diff] [review]
Merge the above changes to the trunk, and share ID matching code between nsHTMLDocument and nsXMLDocument.

>Index: content/base/src/nsDocument.cpp
>===================================================================

>@@ -2449,12 +2451,23 @@ nsDocument::CreateAttribute(const nsAStr
> }
> 
> NS_IMETHODIMP
>-nsDocument::CreateAttributeNS(const nsAString & namespaceURI,
>-                              const nsAString & qualifiedName,
>+nsDocument::CreateAttributeNS(const nsAString & aNamespaceURI,
>+                              const nsAString & aQualifiedName,
>                               nsIDOMAttr **_retval)

Want to use aResult instead of _retval?

>Index: content/html/document/src/nsHTMLDocument.cpp
>===================================================================

>@@ -1327,6 +1372,9 @@ nsHTMLDocument::GetCompatibilityMode(nsC
> NS_IMETHODIMP
> nsHTMLDocument::SetCompatibilityMode(nsCompatibility aMode)
> {
>+  NS_ASSERTION(!(IsXHTML() && aMode == eCompatibility_FullStandards),
>+               "Bad compat mode for XHTML document!");

Shouldn't that be (aMode == eCompatibility_FullStandards || !IsXHTML(), ...)?

>@@ -1424,9 +1472,8 @@ nsHTMLDocument::AttributeWillChange(nsIC
> {
>   NS_ABORT_IF_FALSE(aContent, "Null content!");
> 
>-  // XXX: Check namespaces!!!
>-
>-  if (aAttribute == nsHTMLAtoms::name) {
>+  if (!IsXHTML() && aAttribute == nsHTMLAtoms::name &&
>+      aNameSpaceID == kNameSpaceID_None) {

The XHTML spec just says name is deprecated, do we really want to check for
!IsXHTML already?

>@@ -2340,8 +2434,15 @@ nsresult
> nsHTMLDocument::OpenCommon(nsIURI* aSourceURL)
> {
>   // If we already have a parser we ignore the document.open call.
>-  if (mParser)
>+  if (mParser) {
>+    if (IsXHTML()) {
>+      // No calling document.open() while we're parsing XHTML

Why?

>@@ -3709,6 +3791,12 @@ nsHTMLDocument::ResolveName(const nsAStr

>+  if (IsXHTML()) {
>+    // We don't dynamically resolve names on XHTML documents.
>+
>+    return NS_OK;
>+  }
>+
...
>@@ -3740,8 +3828,8 @@ nsHTMLDocument::ResolveName(const nsAStr
>     entry->mContentList = list;
>     NS_ADDREF(entry->mContentList);
> 
>-    if(mRootContent && !aName.IsEmpty()) {
>-      FindNamedItems(aName, mRootContent, *entry);
>+    if (mRootContent && !aName.IsEmpty()) {
>+      FindNamedItems(aName, mRootContent, *entry, IsXHTML());

Since you return early on IsXHTML you can change the last argument to PR_FALSE

>Index: extensions/transformiix/source/xslt/txMozillaXSLTProcessor.cpp
>===================================================================

>@@ -180,13 +179,19 @@ txToFragmentHandlerFactory::createHandle

>+            if (doc && !doc->IsCaseSensitive()) {
>+                format.mMethod = eHTMLOutput;
>+            } else {
>+                format.mMethod = eXMLOutput;
>+            }

Just make this:

	    if (!doc || doc->IsCaseSensitive()) {
		format.mMethod = eXMLOutput;
	    } else {
		format.mMethod = eHTMLOutput;
	    }

Fix these:
http://www.johnkeiser.com/cgi-bin/jst-review-cgi.pl?patch_url=http%3A%2F%2Fbugz
illa.mozilla.org%2Fattachment.cgi%3Fid%3D119355&patch_text=&reason_type=N&reaso
n_type=W

The only thing I'm not sure of is the output handlers, we're now asserting that
a document that's case-insensitive is an HTML document. What if we (god forbid
;-)) support another case-insensitive documenttype?

    
    

    
  
Intuition says that compatibility mode should Not be an issue. We should use
full-standards mode. (Valid HTML is a prereq to content negotiation. He who uses
content negotiation  is not serving tag soup.

+  NS_ASSERTION(!(IsXHTML() && aMode == eCompatibility_FullStandards),
+               "Bad compat mode for XHTML document!");

Nice, unfortunately, XHTML 1.0 Transitional document type declaration with an
XML declaration is currently rendered in standards mode, Not full standards
mode. Effectively, you're saying "If it's an XHTML transitional doctype, the
content-type determines the rendering mode." 



+    if (IsXHTML()) {
+      // No calling document.open() while we're parsing XHTML
+
+      return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
+    }

HTML DOM supports document.write(), and document.write() calls document.open(),
so document.open() must be supported.

+  } else if (IsXHTML()) {
+    // No calling document.write*() while parsing XHTML!
+
+    return NS_ERROR_DOM_NOT_SUPPORTED_ERR;
   }

Here, too. Need to support write(). See my comment 30, comment 58, comment 72.
My answer to comment 30 is bug 192367.

    
    

    
  
+  NS_ASSERTION(!(IsXHTML() && aMode == eCompatibility_FullStandards),
+               "Bad compat mode for XHTML document!");

This just looks wrong... This assert will fire if IsXHTML() is true and aMode ==
eCompatibility_FullStandards.  Did you mean to have a != there instead of ==?

> so document.open() must be supported.

Making that work with the XML parser is a huge undertaking... (and doomed to
failure, imo, since it would simply lead to invalid documents in 99% of cases)
and as jst pointed out the other day, NOTHING in the DOM HTML spec says that
document.open()/document.write() can be used while the document is loading.  If
you note, we will allow document.open()/document.write() on a document that is
_not_ currently in the middle of being parsed.

    
    

    
  
>we will allow document.open()/document.write() on a document that is
_not_ currently in the middle of being parsed.

What would be the use of that? 


As far as my development goes, I wouldn't get any benefit from such functionality. 


If document.write leads to invalid markup, provide a parse error. DOM methods
for creating and adding elements and css methods are powerful but do not work
cross-browser. The amount of code required to add a simple css rule to IE and
Mozilla and NS4 requires too much work. 

I've grown accustomed to writing stuff such as

document.writeln("<style type='text/css'>/*<![CDATA[*/\n.menu{ visibility:
hidden }\n/*]]>*/</"+"style">");

if(DOM) {

}
else if(IE) { // may be able to eliminate this test. IE supports write().

}

else if(NS4){

}


Creating a cross browser function to add this requires over 10 times the amount
of code.

A question for Boris: How much performance would we gain by dropping write/open?
What is the maintenance issue with that? 

    
    

    
  
> What would be the use of that? 

You could created documents in iframes via document.write; a common practice.

> If document.write leads to invalid markup, provide a parse error

This is highly nontrivial to do.  It may happen sometime, but _not_ in this patch.

There is already a separate bug on document.write.  DO NOT pollute this bug with
that argument any more, please.


    
    

    
  
We currently do NOT support name attributes for XHTML documents parsed as XML,
so I don't think we should start supporting them now.

    
    

    
  
The name attribute is deprecated in XHTML 1.0, and is not present in XHTML 1.1.
Not supporting it for documents parsed as XML seems perfectly reasonable.

    
    

    
  
> You could created documents in iframes via document.write;

An iframe's contentDocument is an HTMLDocument by default. The way things are
now, you can call write on an iframe. Even from an xml document. The only time
this is not true is when the iframe has loaded an xml document for its
contentDocument. Who cares about that? I know I don't. Wouldn't be very hard to
code around that, if I did have such problem.

document.write is the primary reason I care about this bug. It's the only thing
that cannot be easily usurped. 

>Not supporting it for documents parsed as XML seems perfectly reasonable.

Who needs radio buttons, checkboxes, anyway?

No but seriously, we need the name attribute for radios and checkboxes, don't we? 

    
    

    
  
Sure, we are not _prohibiting_ name attributes. Using a name attribute on some
form controls should still send the name to the server. The issue is, will
getElementById(), CSS #id selector and so on find it, and the answer should
remain no.

    
    

    
  
> document.write is the primary reason I care about this bug.

Then you're caring about the wrong bug.

    
    

    
  
Exactly, this will *not* make it possible to call document.write() on a XHTML
document while the document is being loaded. I don't think we'll ever support
that...

    
    

    
  
I have been informed that DOM3 has the getFeature() call, which makes my
concerns about what interfaces are supported on the root element moot:

   http://www.w3.org/TR/DOM-Level-3-Core/core.html#DOMImplementation3-getFeature

I therefore no longer object to the proposed fix, nor to marking this bug FIXED
once that patch is checked in.

    
    

    
  
Comment on attachment 119355 [details] [diff] [review]
Merge the above changes to the trunk, and share ID matching code between nsHTMLDocument and nsXMLDocument.

>+#define XML_DECLARATION_BITS_DECLARATION_EXISTS   1
>+#define XML_DECLARATION_BITS_ENCODING_EXISTS      2
>+#define XML_DECLARATION_BITS_STANDALONE_EXISTS    4
>+#define XML_DECLARATION_BITS_STANDALONE_YES       8

I think some people prefer (1<<0), (1<<1), (1<<2) etc. Not a big deal for me.

>@@ -821,7 +828,23 @@ nsHTMLDocument::StartDocumentLoad(const 
>                                   PRBool aReset,
>                                   nsIContentSink* aSink)
> {
>-  PRBool needsParser=PR_TRUE;
>+  nsCAutoString contentType;
>+  aChannel->GetContentType(contentType);
>+
>+  if (contentType.Equals("application/xhtml+xml")) {
>+    // We're parsing XHTML as XML, remember that.
>+
>+    mNamespaceID = kNameSpaceID_XHTML;

Hmm.. this could get confusing since document-nodes doesn't really have
namespaces. I would prefer a |PRBool mIsXHTML| member instead.

>@@ -1327,6 +1372,9 @@ nsHTMLDocument::GetCompatibilityMode(nsC
> NS_IMETHODIMP
> nsHTMLDocument::SetCompatibilityMode(nsCompatibility aMode)
> {
>+  NS_ASSERTION(!(IsXHTML() && aMode == eCompatibility_FullStandards),
>+               "Bad compat mode for XHTML document!");
>+

As someone pointed out, this seems inverted.

> NS_IMETHODIMP
> nsHTMLDocument::AttributeChanged(nsIContent* aContent, PRInt32 aNameSpaceID,
>-                                 nsIAtom* aAttribute, PRInt32 aModType, nsChangeHint aHint)
>+                                 nsIAtom* aAttribute, PRInt32 aModType,
>+                                 nsChangeHint aHint)
> {
>   NS_ABORT_IF_FALSE(aContent, "Null content!");
> 
>   // XXX: Check namespaces!
> 
>-  if (aAttribute == nsHTMLAtoms::name) {
>+  if (!IsXHTML() && aAttribute == nsHTMLAtoms::name &&
>+      aNameSpaceID == kNameSpaceID_None) {

Remove the namespace comment?

> NS_IMETHODIMP
> nsHTMLDocument::CreateElementNS(const nsAString& aNamespaceURI,
>                                 const nsAString& aQualifiedName,
>@@ -1570,20 +1627,23 @@ nsHTMLDocument::CreateElement(const nsAS
>   NS_ENSURE_TRUE(!aTagName.IsEmpty(), NS_ERROR_DOM_INVALID_CHARACTER_ERR);
> 
>   nsCOMPtr<nsINodeInfo> nodeInfo;
>+
>   nsAutoString tmp(aTagName);
>-  ToLowerCase(tmp);
> 
>-  mNodeInfoManager->GetNodeInfo(tmp, nsnull, kNameSpaceID_None,
>-                                *getter_AddRefs(nodeInfo));
>+  if (!IsXHTML()) {
>+    ToLowerCase(tmp);
>+  }
>+
>+  nsresult rv = mNodeInfoManager->GetNodeInfo(tmp, nsnull, mNamespaceID,
>+                                              *getter_AddRefs(nodeInfo));
>+  NS_ENSURE_SUCCESS(rv, rv);

Ideally you should for XHTML use tmp as a qualified-name rather then a
local-name (I think). However there is not really any good function in the
nodeinfomanager that can do that.

>   nsCOMPtr<nsIHTMLContent> content;
>-  nsresult rv = NS_CreateHTMLElement(getter_AddRefs(content), nodeInfo,
>-                                     PR_FALSE);
>-  if (NS_SUCCEEDED(rv)) {
>-    content->SetContentID(mNextContentID++);
>-    rv = CallQueryInterface(content, aReturn);
>-  }
>-  return rv;
>+  rv = NS_CreateHTMLElement(getter_AddRefs(content), nodeInfo, PR_FALSE);
>+  NS_ENSURE_SUCCESS(rv, rv);

The last argument to NS_CreateHTMLElement should be IsXHTML()

>+PRInt32
>+GetDocumentNamespace(nsIContent *aContent)

Nit: maybe GetHTMLDocumentNamespace would be a better name?

>@@ -3867,7 +3955,7 @@ nsHTMLDocument::GetBodyContent()
>       nsCOMPtr<nsINodeInfo> ni;
>       child->GetNodeInfo(*getter_AddRefs(ni));
> 
>-      if (ni->Equals(nsHTMLAtoms::body)) {
>+      if (ni->Equals(nsHTMLAtoms::body) && ni->NamespaceEquals(mNamespaceID)) {

Why not |ni->Equals(nsHTMLAtoms::body, mNamespaceID)|?


>@@ -180,13 +179,19 @@ txToFragmentHandlerFactory::createHandle
>         {
>             txOutputFormat format;
>             format.merge(*aFormat);
>-            nsCOMPtr<nsIDOMDocument> doc;
>-            mFragment->GetOwnerDocument(getter_AddRefs(doc));
>-            NS_ASSERTION(doc, "unable to get ownerdocument");
>+            nsCOMPtr<nsIDOMDocument> domdoc;
>+            mFragment->GetOwnerDocument(getter_AddRefs(domdoc));
>+            NS_ASSERTION(domdoc, "unable to get ownerdocument");
>             // Need a way for testing xhtml vs. html. But this is the best
>             // we can do for now.

IMHO you can remove this comment since this is the final way to test
xhtml/html, right?

with that r=sicking

        Attachment #119355 -
        Flags: review?(bugmail) → review+

    
    

    
  
I made all those changes, except a few. Here's responces to the above comments
that I didn't fix in the patch.

Peterv said:

> The XHTML spec just says name is deprecated, do we really want to check for
> !IsXHTML already?

This only means that we won't mape images, form controls, and so on, by their
name into the document namespace. I.e. you can't do document.foo.src=..., you'll
need to do document.images.foo.src=..., as you should've been doing in the first
place.

> >+      // No calling document.open() while we're parsing XHTML
> 
> Why?

Because we don't support document.open()/write() et al on XHTML documents. Maybe
we will, to some degree, but that outside the scope of this bug.

> The only thing I'm not sure of is the output handlers, we're now asserting
> that a document that's case-insensitive is an HTML document. What if we
> (god forbid ;-)) support another case-insensitive documenttype?

We'll deal with that problem when we get there, but I doubt we ever will :-)

Garrett Smith wrote:

> +  NS_ASSERTION(!(IsXHTML() && aMode == eCompatibility_FullStandards),
> +               "Bad compat mode for XHTML document!");
> 
> Nice, unfortunately, XHTML 1.0 Transitional document type declaration with
> an XML declaration is currently rendered in standards mode, Not full
> standards mode. Effectively, you're saying "If it's an XHTML transitional
> doctype, the content-type determines the rendering mode." 

Exactly, XHTML means fully standard mode. The only thing with transitional
documents that's not fully standard is some parsing tweaks we have in the HTML
parser, the rest is same as fully standard, thus, since we're not using the HTML
parser, we can set all XHTML documents in fully standards mode.

> HTML DOM supports document.write(), and document.write() calls
> document.open(), so document.open() must be supported.

As we've repeatedly said here, it won't be, at least not yet (and not in this bug).

Sicking said:

> >+    mNamespaceID = kNameSpaceID_XHTML;
> 
> Hmm.. this could get confusing since document-nodes doesn't really have
> namespaces. I would prefer a |PRBool mIsXHTML| member instead.

I renamed this to mDefaultNamespaceID, better?

> Ideally you should for XHTML use tmp as a qualified-name rather then a
> local-name (I think). However there is not really any good function in
> the nodeinfomanager that can do that.

No, we don't want that. CreateElement always creates a prefix-less element,
which can have a ':' in its name.

    
    

    
  
Fix checked in. Marking FIXED.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED

        Attachment #119355 -
        Flags: superreview?(peterv)

    
    

    
  
jst, hixie: should we reopen and restore this bug to app/xml and text/xml?

For that issue, I created a discussion blog entry at
http://www.mozillazine.org/weblogs/weirdal/archives/002975.html

So commentary on this bug may not be necessary at this time.

    
    

    
  
IMHO we should not. You can always get whatever interface you want using
getFeature in DOM3. The XML MIME types are pure XML, I think they should default
to an XMLDocument and if you want an HTML one you should use getFeature.

    
  
Blocks: 78128

    
    

    
  
In my humble opinion, I don't think checking for the MIME type
(application/xhtml+xml) is a great idea.  However, I don't think checking for
the default namespace is:

...
<foo:bar xmlns="http://www.w3.org/1999/xhtml" xmlns:foo="urn:something/else">
...

XHTML is the default NS, but bar is the root element in the foo NS.  You can't
depend on the namespace alone.

However, DOCTYPE should be enough to determine that a HTML document is an HTML
document (in addition to MIME type)



    
    

    
  
I just filed bug 340017, "document.formName doesn't work in XHTML".  Bug 109373 covered that and was marked as a dup of this bug.

    
    

    
  
See also bug 349308, "HTML DOM is not available in XHTML documents sent as application/xml".

    
  
Component: DOM: HTML → DOM: Core & HTML
QA Contact: stummala → general

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: