Closed Bug 623372 Opened 14 years ago Closed 14 years ago

Node type and namespace fields get in the way when using the Insert Node dialog

Categories

(Other Applications :: DOM Inspector, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: crussell, Assigned: crussell)

References

Details

Attachments

(1 file, 1 obsolete file)

In some bug, Neil made mention (which I can no longer find) of the namespace field in the Insert Dialog defaulting to something reasonable. Additionally, I've found that my use case for the Insert Node dialog is overwhelmingly to insert a node which is: i) an element, and ii) in the same namespace as the reference node. I think that it's probably true for most others as well. The dialog should default to inserting an element and focus the name field, so when the dialog appears you will be able to type the name and press enter to handle almost all use cases. Inserting a text node is so rare that when that's what you really want to do, you can tab to the Node Type menulist or click on it and select the appropriate item. This patch also removes the restriction on specifying the namespace when the document content-type is "text/html" because: i) I couldn't work out how the affected code in initialize was supposed to deal with enableNamespaces being false, and ii) it seems that disallowing the namespace for "text/html" is in error. A side effect of the changes in this patch is that the Custom namespace menuitem also now remembers the textbox value, so if you select another menuitem after typing a custom namespace in the textbox, then reselect the Custom menuitem, the textbox value is reset to what you had originally typed.
Attachment #501497 - Flags: review?(neil)
(In reply to comment #0) > This patch also removes the restriction on specifying the namespace when the > document content-type is "text/html" because: > i) I couldn't work out how the affected code in initialize was supposed to > deal with enableNamespaces being false, and > ii) it seems that disallowing the namespace for "text/html" is in error. Well, I believe that it makes no difference any more, but I think there was some edge case between createElement and createElementNS at some point.
This all was added in bug 205872 (which may be the first Mozilla bug I ever fixed...). Might be history there that is worth reading.
It looks like the patches for that bug are about the DOM Nodes viewer.
(In reply to comment #3) > It looks like the patches for that bug are about the DOM Nodes viewer. I think I misread this bug. My bad!
(In reply to comment #1) > (In reply to comment #0) > > This patch also removes the restriction on specifying the namespace when the > > document content-type is "text/html" because: > > i) I couldn't work out how the affected code in initialize was supposed to > > deal with enableNamespaces being false, and > > ii) it seems that disallowing the namespace for "text/html" is in error. > Well, I believe that it makes no difference any more, but I think there was > some edge case between createElement and createElementNS at some point. [Note: I don't know when the switch occurred, but my understanding is that DOMi supports both Gecko 1.9.1 and Gecko 2 so that suffices for this purpose.] The problem with Gecko 1.9.1 is that HTML elements have a blank namespaceURI but you can't call createElementNS with a blank namespaceURI because you get a non-namespaced element rather than an (X)HTML element. And even with Gecko 2 you have to be careful because although all (X)HTML nodes have a namespaceURI, createElement creates a case-insensitive HTML element but createElementNS creates a case-sensitive XHTML element. [This discussion only applies to text/html documents of course; XML (XHTML, XUL, XBL, SVG etc.) documents can only contain XHTML elements.]
Comment on attachment 501497 [details] [diff] [review] Default to deriving the namespace from originalNode for Insert; remember namespaces for Custom; allow namespaces for "text/html" >+ var menulist = this.menulist; >+ var uri = this.mData.namespaceURI; >+ if (uri) { >+ menulist.value = uri; >+ if (!menulist.selectedItem) { >+ this.customNS.value = uri; >+ menulist.selectedItem = this.customNS; >+ } >+ } IMHO it would be better to write this as this.menulist.value = this.customNS.value = uri;
Comment on attachment 501497 [details] [diff] [review] Default to deriving the namespace from originalNode for Insert; remember namespaces for Custom; allow namespaces for "text/html" r- as per comment #5 for not supporting createElement.
Attachment #501497 - Flags: review?(neil) → review-
(In reply to comment #6) > Comment on attachment 501497 [details] [diff] [review] > Default to deriving the namespace from originalNode for Insert; remember > namespaces for Custom; allow namespaces for "text/html" > > >+ var menulist = this.menulist; > >+ var uri = this.mData.namespaceURI; > >+ if (uri) { > >+ menulist.value = uri; > >+ if (!menulist.selectedItem) { > >+ this.customNS.value = uri; > >+ menulist.selectedItem = this.customNS; > >+ } > >+ } > IMHO it would be better to write this as > this.menulist.value = this.customNS.value = uri; The problem there is that it behaves in a weird way when the namespace matches one of the items in the menulist.
Attachment #501497 - Attachment is obsolete: true
Attachment #516989 - Flags: review?(neil)
(In reply to comment #8) > (In reply to comment #6) > > (From update of attachment 501497 [details] [diff] [review]) > > >+ var menulist = this.menulist; > > >+ var uri = this.mData.namespaceURI; > > >+ if (uri) { > > >+ menulist.value = uri; > > >+ if (!menulist.selectedItem) { > > >+ this.customNS.value = uri; > > >+ menulist.selectedItem = this.customNS; > > >+ } > > >+ } > > IMHO it would be better to write this as > > this.menulist.value = this.customNS.value = uri; > The problem there is that it behaves in a weird way when the namespace matches > one of the items in the menulist. Could you be more specific? I didn't notice anything weird when I tried it.
(In reply to comment #9) > (In reply to comment #8) > > (In reply to comment #6) > > > (From update of attachment 501497 [details] [diff] [review]) > > > >+ var menulist = this.menulist; > > > >+ var uri = this.mData.namespaceURI; > > > >+ if (uri) { > > > >+ menulist.value = uri; > > > >+ if (!menulist.selectedItem) { > > > >+ this.customNS.value = uri; > > > >+ menulist.selectedItem = this.customNS; > > > >+ } > > > >+ } > > > IMHO it would be better to write this as > > > this.menulist.value = this.customNS.value = uri; > > The problem there is that it behaves in a weird way when the namespace matches > > one of the items in the menulist. > Could you be more specific? I didn't notice anything weird when I tried it. From an XHTML node, set the menulist to something like XMLNS. Now switch it to Custom. The contents of the namespace textbox reverts to the XHTML namespace. That's weird to me. I'd expect the textbox value to either a) not change at all except for being enabled, or b) be the empty string.
(In reply to comment #10) > That's weird to me. I'd expect the textbox value to either a) not change at > all except for being enabled, or b) be the empty string. Fair enough. Personally I thought that the parent node's name space was a reasonable default for the custom name space.
Attachment #516989 - Flags: review?(neil) → review+
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: