847195 - Make NamedNodeMap only deal with Attrs

Status: RESOLVED FIXED

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

Description

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

Attachment: Patch v1

Once Attr no longer inherits from Node, we won't we able to QI back and forth like the callers here do.

Comment 1

[Mounir Lamouri (:mounir)]

Comment on attachment 720439 [details] [diff] [review]
Patch v1

Don't have time nor motivation for that.

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 720439 [details] [diff] [review]
Patch v1

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

Please mark nsIDOMAttr as builtinclass.

::: content/base/src/nsDOMAttributeMap.cpp
@@ +200,3 @@
>  {
>    ErrorResult rv;
> +  *aReturn = SetNamedItemInternal(aAttr, false, rv).get();

Wow this code is ugly

::: dom/interfaces/core/Makefile.in
@@ +23,5 @@
>  	nsIDOMDocument.idl			\
>  	nsIDOMDocumentFragment.idl		\
>  	nsIDOMDocumentType.idl			\
>  	nsIDOMElement.idl			\
> +	nsIDOMMozNamedAttrMap.idl			\

Line up the \ please.

::: editor/libeditor/base/nsEditor.cpp
@@ +2241,3 @@
>      // always remove item number 0 (first item in list)
> +    if (NS_SUCCEEDED(destAttributes->Item(0, getter_AddRefs(attr))) && attr) {
> +      nsAutoString str;

Don't use nsAutoString here.  The stack buffer is wasted.  nsString is just fine.  (Yes I realize this was here before).

@@ +2258,3 @@
>    {
> +    if (NS_SUCCEEDED(sourceAttributes->Item(i, getter_AddRefs(attr))) && attr) {
> +      nsAutoString sourceAttrName;

Same here.

@@ +2258,5 @@
>    {
> +    if (NS_SUCCEEDED(sourceAttributes->Item(i, getter_AddRefs(attr))) && attr) {
> +      nsAutoString sourceAttrName;
> +      if (NS_SUCCEEDED(attr->GetName(sourceAttrName))) {
> +        nsAutoString sourceAttrValue;

And here.

@@ +2278,1 @@
>  #endif

You can remove #ifdef DEBUG_personiveneverheardof code.

Comment 3

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

https://hg.mozilla.org/mozilla-central/rev/b1a08130fae6

Comment 4

[Kohei]

https://developer.mozilla.org/en-US/docs/Web/API/NamedNodeMap
https://developer.mozilla.org/en-US/docs/Site_Compatibility_for_Firefox_22

Comment 5

[Masatoshi Kimura [:emk]]

(In reply to Kohei Yoshino [:kohei] from comment #4)
> https://developer.mozilla.org/en-US/docs/Web/API/NamedNodeMap
> https://developer.mozilla.org/en-US/docs/Site_Compatibility_for_Firefox_22

The interface has been renamed to MozNamedAttrMap, not MozNamedNodeMap. (Because this interface is only used for attributes.)

Comment 6

[Kohei]

(In reply to Masatoshi Kimura [:emk] from comment #5)
> The interface has been renamed to MozNamedAttrMap, not MozNamedNodeMap.
> (Because this interface is only used for attributes.)

Thank you for pointing it out. I just fixed the documents.