Update DOMException name: HierarchyRequestError

[Erik Arvidsson]

Update DOMException name: HierarchyRequestError

[Erik Arvidsson]

Created attachment 173902 [details]
Patch

[Ojan Vafai]

Comment on attachment 173902 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=173902&action=review

> Source/WebCore/dom/DOMCoreException.cpp:40
> +    { "HierarchyRequestError", "A Node was inserted somewhere it doesn't belong." },

Sigh. Not directly related to your patch, but this is not the most accurate error message. As in, the error below is a subset of this. It should probably be something like "Node cannot be inserted into one of it's ancestors." I'm not 100% sure we consistently only use it in this way, but we should. :)

[Erik Arvidsson]

(In reply to comment #2)
> (From update of attachment 173902 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=173902&action=review
> 
> > Source/WebCore/dom/DOMCoreException.cpp:40
> > +    { "HierarchyRequestError", "A Node was inserted somewhere it doesn't belong." },
> 
> Sigh. Not directly related to your patch, but this is not the most accurate error message. As in, the error below is a subset of this. It should probably be something like "Node cannot be inserted into one of it's ancestors." I'm not 100% sure we consistently only use it in this way, but we should. :)

The description of these are part of the standard too now. I plan to get to that in bug 101953

[WebKit Review Bot]

Comment on attachment 173902 [details]
Patch

Clearing flags on attachment: 173902

Committed r134435: <http://trac.webkit.org/changeset/134435>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

I suggest we also rename our internal HIERARCHY_REQUEST_ERR constant. The only reason it currently has the name it has is that we followed what was in the DOM specification.

[Erik Arvidsson]

(In reply to comment #6)
> I suggest we also rename our internal HIERARCHY_REQUEST_ERR constant. The only reason it currently has the name it has is that we followed what was in the DOM specification.

I'm not sure which internal constant you are referring to? The DOMException code property still reflects this numeric constant and the IDL file exposes constants for these.

[Darin Adler]

I’m mixed up here. I guess the name HIERARCHY_REQUEST_ERR still has relevance because it’s the name in the “name” property?

[Erik Arvidsson]

(In reply to comment #8)
> I’m mixed up here. I guess the name HIERARCHY_REQUEST_ERR still has relevance because it’s the name in the “name” property?

I understand this is confusing. The "name" property of the DOMExceptio is supposed to be "HierarchyRequestError". The "code" property will continue to be 3, which is also reflected as

const unsigned short HIERARCHY_REQUEST_ERR = 3;

in DOMCoreException.idl.

try {
  document.appendChild(document);
} catch (ex) {
  asssert(ex.name === "HierarchyRequestError");
  assert(ex.code === DOMException.HIERARCHY_REQUEST_ERR);
}

Does that resolve your concerns?

[Darin Adler]

Sure, kinda sad that there has to be both an ugly and non-ugly version of each name.

[Csaba Osztrogonác]

(In reply to comment #4)
> (From update of attachment 173902 [details])
> Clearing flags on attachment: 173902
> 
> Committed r134435: <http://trac.webkit.org/changeset/134435>

More expected update landed in http://trac.webkit.org/changeset/134549