Bug 102092

Created attachment 173902 [details] Patch

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. :)

(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

Comment on attachment 173902 [details] Patch Clearing flags on attachment: 173902 Committed r134435: <http://trac.webkit.org/changeset/134435>

All reviewed patches have been landed. Closing bug.

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.

(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.

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

(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?

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

(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