From Bugzilla Helper: User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.0rc3) Gecko/20020523 BuildID: 20020523 In DOM inspector, Tab "Object" > "Object - DOM Node", we have "nodeName" and "nodeValue" as column headers, but IMHO, "nodeName" is quite confusing. I think "Node Attribute", "Attribute" or "Attribute name" are more appropriate but by "nodeName", I would think of the DOM property "nodeName" which gives the name of element. Similarly, "Attribute Value" or "Value" are more appropriate than "nodeValue". Reproducible: Always

Those columns _do_ list the nodeName and nodeValue properties of the attribute nodes... but agreed that the proposed phrasing is a lot clearer.

Mass re-assigning bugs to dom.inspector@extensions.bugs

Change from nodeValue to "Atribute name" and from nodeValue to "Atribute value"?

(In reply to Javi Rueda from comment #3) > Change from nodeValue to "Atribute name" > and from nodeValue to "Atribute value"? No, from nodeName to "Atribute name" and from nodeValue to "Atribute value"

Column header labels are hard-coded, although in the corresponding .dtd file there exists a friendly localizable label. In http://mxr.mozilla.org/comm-central/source/mozilla/extensions/inspector/resources/content/viewers/domNode/domNode.xul#83 it is stated that there is not need to use that label. The patch is easy, but is still needed?

I'm not sure how to answer your question. Let me elaborate. At the time this bug was reported (in 2002!), DOM Inspector was a part of Firefox. Now DOM Inspector is an add-on. Nevertheless, I've just tried DOM Inspector 2.0.10 and those confusing labels are still there. So, as far as those labels are concerned, yes, the patch is needed. But I have no answer as to whether it is that particular label in that particular file (domNode.xul) that we need to change.

(In reply to 石庭豐 (Seak, Teng-Fong) from comment #6) > > But I have no answer as to whether it is that particular label in that > particular file (domNode.xul) that we need to change. The label is used everywhere. For example, it appears in the dialog box for insert a node. My first approach was to change both in domNode.dtd, removing the trailing ":" (and adding it to the corresponding XUL description for said dialog box), also referencing to the .label in domNode.xul. This worked in my compiled XPI. Other way was to hard-code the strings for the column headers. As those strings are used for 2 different things -headers and labels for text-edit boxes- maybe some specific headerNodeValue and headerNodeName could be created only for this use. This way only the "problematic" files would be modified.

As long as those labels are changed for final users (including me), whatever approach is good for me :) Thanks

Could you take a look at this or shifting to a proper reviewer, Neil? Thank you. Changing column headers in Object Viewer. Creates new strings as this labels are only being used here. Localization will be needed.

Same comment as obsoleted previous patch (I didn't refreshed changes to files, sorry).

Comment on attachment 570269 [details] [diff] [review] Changing headers strings in Object Viewer I don't think I'm the right reviewer for this. (But if you want my opinion, then you'd have to change all of the labels.)

Comment on attachment 570269 [details] [diff] [review] Changing headers strings in Object Viewer Review of attachment 570269 [details] [diff] [review]: ----------------------------------------------------------------- Yeah, we should change all the treecol labels away from being IDL-centric, not just the name and value columns. ::: resources/locale/en-US/viewers/domNode.dtd @@ +42,5 @@ > <!ENTITY namespaceURI.label "Namespace URI:"> > <!ENTITY nodeType.label "Node Type:"> > <!ENTITY nodeValue.label "Node Value:"> > +<!ENTITY nodeNameColHeader.label "Attribute Name"> > +<!ENTITY nodeValueColHeader.label "Attribute Value"> The problem with changing all of them like this, though, means that they will all start with "Attribute...", which is pretty redundant and may require more space to display than what's available, depending on window size. It may be better to just put the attribute tree inside a groupbox with an "Attributes" caption. Let me know if you need me to explain further. Also, please change "nodeNameColHeader" to something like just "attributeName".

(In reply to Colby Russell :crussell from comment #12) > Also, please change "nodeNameColHeader" to something like just > "attributeName". Yes, I agree with Colby that it's better to avoid strings beginning with "node..." so that other programmers won't be confused.

Changes the attribute header column labels and puts the tree into a groupbox (which will be hidden when appropiate). Reformatting the code affected by the inserted groupbox. Translation needed, so removing the directories in the Makefile.

Comment on attachment 570727 [details] [diff] [review] Changing columns header Review of attachment 570727 [details] [diff] [review]: ----------------------------------------------------------------- ::: resources/locale/en-US/viewers/domNode.dtd @@ +42,5 @@ > <!ENTITY namespaceURI.label "Namespace URI:"> > <!ENTITY nodeType.label "Node Type:"> > <!ENTITY nodeValue.label "Node Value:"> > +<!ENTITY attributeName.label "Name"> > +<!ENTITY attributeValue.label "Value"> These should either stay hardcoded in the XUL and be lowercased (to reflect the names from IDL), or all treecol headers should be made localizable, properly capitalized, natural language strings (vs. the names from IDL, per comment 12).

I'm referring to this bit: (From comment #12) > Yeah, we should change all the treecol labels away from being IDL-centric, > not just the name and value columns.

(In reply to Colby Russell :crussell from comment #15) > These should either stay hardcoded in the XUL and be lowercased (to reflect Easier easy this one :-)

Comment on attachment 572238 [details] [diff] [review] Changing columns header v1.1 Review of attachment 572238 [details] [diff] [review]: ----------------------------------------------------------------- I haven't been able to dedicate much time to Mozilla stuff recently. So, sorry for taking so long to get to this. ::: resources/content/viewers/domNode/domNode.xul @@ +73,5 @@ > </rows> > </grid> > > + <groupbox id="grpAttr" flex="1"> > + <caption label="Attributes"/> The treecol labels can be hardcoded because they reflect IDL, but this needs to be localizable. Sorry, I should have caught this before. @@ +86,1 @@ > by DOM APIs --> The rest looks good, but we should extend this comment to mention that even though we're really accessing the |nodeName| and |nodeValue| properties, it's fine because they're guaranteed to match the attributes' |name| and |value|, respectively. (Well, that's true so long as there is such a thing as an attribute node, which is set to change soon. We can deal with that when the time comes, though.)

Making the group caption localizable and changing the commentary.

Comment on attachment 575495 [details] [diff] [review] Patch for bug 147174 v1.2 [Checkin: comment 21] http://hg.mozilla.org/dom-inspector/rev/d1b51e82be02

Leaving it up to you whether or when to close the bug.