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

Status: RESOLVED FIXED

(Other Applications :: DOM Inspector, enhancement)

Description

[Colby Russell :crussell (no longer Mozilla-ing)]

Attachment: Default to deriving the namespace from originalNode for Insert; remember namespaces for Custom; allow namespaces for "text/html"

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.

Comment 1

[[e-mail]]

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

Comment 2

[Shawn Wilsher :sdwilsh]

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.

Comment 3

[Colby Russell :crussell (no longer Mozilla-ing)]

It looks like the patches for that bug are about the DOM Nodes viewer.

Comment 4

[Shawn Wilsher :sdwilsh]

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

Comment 5

[[e-mail]]

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

[[e-mail]]

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 7

[[e-mail]]

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.

Comment 8

[Colby Russell :crussell (no longer Mozilla-ing)]

Attachment: Don't eschew with createElement for "text/html"

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

Comment 9

[[e-mail]]

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

Comment 10

[Colby Russell :crussell (no longer Mozilla-ing)]

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

Comment 11

[[e-mail]]

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

Comment 12

[Colby Russell :crussell (no longer Mozilla-ing)]

Pushed: http://hg.mozilla.org/dom-inspector/rev/1ee66cda9147