840401 - Remove nsIDOMDocumentType

Status: RESOLVED DUPLICATE of bug 1445140

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

Comment 1

[David Zbarsky (:dzbarsky)]

Attachment: Patch

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Tiny bit risky stuff. If dtd is removed, how is a binary addon supposed to create for example
SVG document safely?

Comment 3

[David Zbarsky (:dzbarsky)]

I'm not sure I understand what you're asking.  Why can't addons do the same thing they currently do, but use mozilla::dom::DocumentType instead of nsIDOMDocumentType?

But I did just check addons, and it looks like two of them use Components.interfaces.nsIDOMDocumentType

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Because WebIDL stuff is mostly non-virtual and not properly exposed outside libxul.

Comment 5

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

Comment on attachment 712770 [details] [diff] [review]
Patch

Review of attachment 712770 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/src/generic/DocAccessible.cpp
@@ +381,5 @@
>    nsCOMPtr<nsIXULDocument> xulDoc(do_QueryInterface(mDocumentNode));
>    if (xulDoc) {
>      aDocType.AssignLiteral("window"); // doctype not implemented for XUL at time of writing - causes assertion
>      return NS_OK;
>    } else

Please get rid of this else while you're here

@@ +384,5 @@
>      return NS_OK;
>    } else
>  #endif
> +  if (doc) {
> +    doc->GetDoctype()->GetPublicId(aDocType);

GetDoctype() can return null

::: content/base/src/nsDocument.cpp
@@ +4500,3 @@
>  {
>    MOZ_ASSERT(aDoctype);
> +  nsCOMPtr<nsISupports> doctype = static_cast<nsINode*>(nsIDocument::GetDoctype());

Can you try just making this

nsCOMPtr<nsINode> doctype = nsIDocument::GetDoctype();

?

::: content/base/src/nsGenericDOMDataNode.cpp
@@ +8,1 @@
>   * nsIDOMCDATASection, and nsIDOMProcessingInstruction nodes.

Drop the nsIDOM everywhere?

::: content/base/src/nsGenericDOMDataNode.h
@@ +8,1 @@
>   * nsIDOMCDATASection, and nsIDOMProcessingInstruction nodes.

Same

::: content/base/src/nsINode.cpp
@@ +919,5 @@
>        }
>        case nsIDOMNode::DOCUMENT_TYPE_NODE:
>        {
> +        DocumentType* docType1 = static_cast<DocumentType*>(node1);
> +        DocumentType* docType2 = static_cast<DocumentType*>(node2);

Consider a DocumentType::FromNode() that asserts the NodeType() is correct

::: content/xml/document/src/nsXMLDocument.cpp
@@ +142,5 @@
>    // unlike the legacy HTML mess
>    doc->SetDocumentCharacterSet(NS_LITERAL_CSTRING("UTF-8"));
>    
>    if (aDoctype) {
> +    mozilla::ErrorResult res;

Add a "using namespace mozilla;" instead

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 712770 [details] [diff] [review]
Patch

AFAIK after this change there isn't any sane way to create certain types of
data documents. Is that what we really want at this point?
Could we wait a bit once we have more stuff converted to webidl stuff and
then try to figure out some generic solution.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

I'm not aware of such generic solution or plan, but perhaps bz and peterv have some ideas.

Comment 8

[Boris Zbarsky [:bzbarsky]]

I don't think we have one.  We need to either expose some sort of C++ bindings or just make anything we want callable from outside libxul either virtual or exported, right?

Comment 9

[Adrian Wielgosik [:adrian17]]

bug 1445140 removed this interface.