352770 - DOMI DOM Node textboxes should not appear editable

Status: RESOLVED FIXED

(Other Applications :: DOM Inspector, defect)

Description

[Jason Barnabe (np)]

The current textboxes under the DOM Node view look like they are editable, but they aren't. We want to make them look not editable but still allow selection.

Comment 1

[Jason Barnabe (np)]

Attachment: patch v1

Comment 2

[Alex Vincent [:WeirdAl]]

Wait a moment.  Wouldn't it be nicer to let people edit these nodes?

Comment 3

[Jason Barnabe (np)]

I don't think it's feasible to change the node name, type, or namespace. Name and namespace would require creating a new node and moving all the attributes, descendants, and then that node would lose CSS rules and JavaScript properties. Changing the type has all those problems, plus we'd have to check to make sure that the new type would be allowed in that position and that attributes even exist for that type.

Comment 4

[Alex Vincent [:WeirdAl]]

Silly me, I was thinking of textboxes for editing text nodes.  :p  Objection withdrawn.

Comment 5

[Jason Barnabe (np)]

(In reply to comment #4)
> Silly me, I was thinking of textboxes for editing text nodes.  :p  Objection
> withdrawn.
> 

Yeah, I've already fixed that :) (bug 112922)

Comment 6

[[e-mail]]

(In reply to comment #0)
>The current textboxes under the DOM Node view look like they are editable
They do not! Perhaps there's a bug in your theme...

Comment 7

[Jason Barnabe (np)]

(In reply to comment #6)
> (In reply to comment #0)
> >The current textboxes under the DOM Node view look like they are editable
> They do not! Perhaps there's a bug in your theme...
> 

The default Linux theme for Firefox?

In any case, making something look like a disabled textbox implies that it will be enabled in some situations. This patch just changes the UI to match what we do in similar situations on other places, like Page Info.

Comment 8

[[e-mail]]

Comment on attachment 238577 [details] [diff] [review]
patch v1

I guess the class="plain" changes are OK then, but I don't see page info adding extra padding, and using descendant selectors is wrong anyway.

Comment 9

[Jason Barnabe (np)]

Page Info puts a separator column with a width of 0.5em between the two. I figured this way's better. Why are descendant selectors wrong?

I added the padding at the bottom because it looked scrunched against the tree without it. The previous code didn't have this problem because the textbox was taller without class="plain".

Comment 10

[timeless]

Comment on attachment 238577 [details] [diff] [review]
patch v1

mozilla/extensions/inspector/resources/skin/modern/viewers/domNode/domNode.css 	1.6
mozilla/extensions/inspector/resources/skin/classic/viewers/domNode/domNode.css 	1.6
mozilla/extensions/inspector/resources/content/viewers/domNode/domNode.xul 	1.17

Comment 11

[[e-mail]]

(In reply to comment #9)
>Why are descendant selectors wrong?
http://developer.mozilla.org/en/docs/Writing_Efficient_CSS#Avoid_the_descendant_selector.21

Comment 12

[Jason Barnabe (np)]

Attachment: css tweak

Switch from a descendant selector to a class selector.

Comment 13

[Jason Barnabe (np)]

Since timeless didn't care about the CSS, I figure I don't need his approval for this small change.

Comment 14

[Alex Vincent [:WeirdAl]]

That's not how things work.  SR is never a substitute for review.

Comment 15

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #14)
> That's not how things work.  SR is never a substitute for review.

The change is trivial, and I don't agree with your statement that a super-review is never equivalent to a review; that depends entirely on the reviewer. Demanding that the patch go through another round of review in this case is ridiculous.

Comment 16

[:Gavin Sharp [email: [e-mail]]]

extensions/inspector/resources/skin/modern/viewers/domNode/domNode.css 	1.7
extensions/inspector/resources/skin/classic/viewers/domNode/domNode.css 1.7
extensions/inspector/resources/content/viewers/domNode/domNode.xul 1.18