181096 - document.isSupported('HTML', '2.0') returns true in XHTML document loaded as text/xml which doesn't support the HTML DOM

Status: RESOLVED WONTFIX

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

Description

[Martin Honnen]

Beside the document.implementation.hasFeature check for the complete DOM
implementation the DOM Level 2 Core introduces the method
  isSupported
for node objects.
When Mozilla loads XHTML documents with Content-Type text/xml then the document
object is not a HTML document meaning document.write or document.images is not
supported.
Therefore I think a check
  document.isSupported('HTML', '2.0')
should return false as the document node doesn't support the HTML DOM.
However with both Netscape 7 and Mozilla 1.2b the check returns true.
I will attach a test case document demonstrating that document.write etc are not
supported but the isSupported check returns true.

Comment 1

[Martin Honnen]

Attachment: test case which checks document.isSupported for the different feature sets and also checks for document.write

Comment 2

[Boris Zbarsky [:bzbarsky]]

Yeah, we should fix this.

Comment 3

[Martin Honnen]

Attachment: test case

The previous test case causes a JavaScript error (although it does nevertheless
demonstrate the probelem). But I attach a new test case here that doesn't cause
the script error while demonstrating the problem.

Comment 4

[Frank Wein [:mcsmurf]]

Ok, that has to be changed AFAIK at
http://lxr.mozilla.org/mozilla/source/content/base/src/nsGenericElement.cpp#1226

Comment 5

[Frank Wein [:mcsmurf]]

Patch is coming soon...

Comment 6

[Frank Wein [:mcsmurf]]

Attachment: Patch 1

Comment 7

[Frank Wein [:mcsmurf]]

.

Comment 8

[Boris Zbarsky [:bzbarsky]]

ccing jst, who owns this shit and so has to see this stuff... ;)

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 106932 [details] [diff] [review]
Patch 1

This is wrong for the following reasons:

1)  We don't support DOM HTML 1.0 in XHTML as XML either.

2)  We _do_ support them both in XHTML as HTML, and your change would say we do
not.

3)  You're changing nsIDOMNode::IsSupported, and non-document nodes in XHTML as
XML fully support all DOM HTML 1 and 2 methods.

Comment 10

[Frank Wein [:mcsmurf]]

ok, sorry this is somewhat complicated to me 
Reassign to default owner

Comment 11

[Christopher Aillon (sabbatical, not receiving bugmail)]

I have other changes in my tree right now which prevent me from making a patch
here, but you want to modify nsDocument::IsSupported() to do something like this:

NS_IMETHODIMP
nsDocument::IsSupported(const nsAString& aFeature,
                        const nsAString& aVersion,
                        PRBool* aReturn)
{
  NS_ENSURE_ARG_POINTER(aReturn);
  nsAutoString feature(aFeature);

  if (feature.EqualsIgnoreCase("HTML") &&
      !mContentType.EqualsIgnoreCase("text/html")) {
    *aReturn = PR_FALSE;
    return NS_OK;
  }

  return nsGenericElement::InternalIsSupported(aFeature, aVersion, aReturn);
}

Comment 12

[Frank Wein [:mcsmurf]]

hm, ok perhaps I'll try it again

Comment 13

[Frank Wein [:mcsmurf]]

Attachment: Patch 2

Here's patch 2, the code from Christopher (thanks!)

Comment 14

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 107153 [details] [diff] [review]
Patch 2

I'd prefer not to base this on the content-type like that (since we may fix our
application/xhtml+xml bug at some point).  How about returning PR_FALSE
unconditionally for the "html" feature here and implementing
"nsHTMLDocument::IsSupported" which returns PR_TRUE?  Since nsHTMLDocument
subclasses nsDocument, that would do the right thing, methinks...

Comment 15

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: Make this work, as well as support the XHTML feature

HTML is defined to be true if both HTML and XML return true.  So the places
that return true for HTML also return true for XHTML, since we always support
XML.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 108339 [details] [diff] [review]
Make this work, as well as support the XHTML feature

+  nsAutoString feature(aFeature);

Don't do that.	In fact, fix the other places doing it.  ;)

+    // XXX we don't support the HTML DOM on HTMLDocuments yet.
+    *aReturn = PR_FALSE;

This part is just confused.... ;)

Comment 17

[Christopher Aillon (sabbatical, not receiving bugmail)]

Brain fart, there.  Okay this is an interesting bug, now.  Currently hasFeature
and isSupported share the same code path, which is similar, but they need to
return different things.  I can make them do the right thing (as long as I'm
thinking clearly) but now I'm just trying to figure out the best way to do that
with minimal duplication of code....

Comment 18

[Garrett Smith]

What about HTMLEvents?

Comment 19

[Garrett Smith]

What about HTMLEvents?

Comment 20

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

No longer relevant with the current implementation of isSupported