Bug 102092

Summary: Update DOMException name: HierarchyRequestError
Product: WebKit Reporter: Erik Arvidsson <arv>
Component: New BugsAssignee: Erik Arvidsson <arv>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, eric.carlson, feature-media-reviews, ossy, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 101606    
Attachments:
Description Flags
Patch none

Erik Arvidsson
Reported 2012-11-13 08:30:52 PST
Update DOMException name: HierarchyRequestError
Attachments
Patch (37.38 KB, patch)
2012-11-13 08:35 PST, Erik Arvidsson
no flags
Erik Arvidsson
Comment 1 2012-11-13 08:35:58 PST
Ojan Vafai
Comment 2 2012-11-13 09:08:43 PST
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
Comment 3 2012-11-13 09:10:39 PST
(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 4 2012-11-13 10:02:48 PST
Comment on attachment 173902 [details] Patch Clearing flags on attachment: 173902 Committed r134435: <http://trac.webkit.org/changeset/134435>
WebKit Review Bot
Comment 5 2012-11-13 10:02:52 PST
All reviewed patches have been landed. Closing bug.
Darin Adler
Comment 6 2012-11-13 10:26:26 PST
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
Comment 7 2012-11-13 10:52:43 PST
(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
Comment 8 2012-11-13 12:25:44 PST
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
Comment 9 2012-11-13 13:11:57 PST
(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
Comment 10 2012-11-13 17:35:42 PST
Sure, kinda sad that there has to be both an ugly and non-ugly version of each name.
Csaba Osztrogonác
Comment 11 2012-11-13 22:58:01 PST
(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
Note You need to log in before you can comment on or make changes to this bug.