729220 - Allow editing of attribute names in the HTML panel of the Page Inspector

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Rob Campbell [:rc] (:robcee)]

We currently only have editing of attribute values in the page inspector. We should allow editing of attribute names as well.

Comment 1

[Rob Campbell [:rc] (:robcee)]

Attachment: an egregious hack wip

It's not pretty, but it's based on some pretty ugly editor code to begin with. I think this has potential to be cleaned up and made less awful.

Editing text nodes should be straight-forward as well, but I will probably do that and tabbing in followups.

Comment 2

[Rob Campbell [:rc] (:robcee)]

asking dcamp for addl feedback.

hoping to get back to this patch this week.

Comment 3

[Paul Rouget [:paul]]

(working on that today)

Comment 4

[Rob Campbell [:rc] (:robcee)]

(ok, but be careful. it's ugly.)

Comment 5

[Dave Camp (:dcamp)]

Comment on attachment 599280 [details] [diff] [review]
an egregious hack wip

Review of attachment 599280 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/devtools/highlighter/TreePanel.jsm
@@ +340,5 @@
> +    if (this.hasClass(target, "nodeName")) {
> +      let attrName = target.innerHTML;
> +      let attrValNode = target.nextSibling.nextSibling; // skip 2 (=)
> +
> +      if (attrValNode)

When is this going to be false?  What is it protecting against?

Comment 6

[Paul Rouget [:paul]]

Comment on attachment 599280 [details] [diff] [review]
an egregious hack wip

f+. The infobar is not updated though (even if we change a value, so it's a more general problem).

Comment 7

[Rob Campbell [:rc] (:robcee)]

(In reply to Dave Camp (:dcamp) from comment #5)
> Comment on attachment 599280 [details] [diff] [review]
> an egregious hack wip
> 
> Review of attachment 599280 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/devtools/highlighter/TreePanel.jsm
> @@ +340,5 @@
> > +    if (this.hasClass(target, "nodeName")) {

> When is this going to be false?  What is it protecting against?

there are a bunch of different classes in the html panel's DOM nodes.

nodeName is specific to the attribute name. nodeValue is for the attributeValue, editGroup for the parent "group" node, etc. It'll be false for any of these as well as text nodes, twisties, ...

nodeName is what makes the attribute's name node a thing and this recognizes that you've clicked one.

Thanks for the feedback, guys. I'll polish this up and get it tested.

Comment 8

[Rob Campbell [:rc] (:robcee)]

(In reply to Paul Rouget [:paul] from comment #6)
> Comment on attachment 599280 [details] [diff] [review]
> an egregious hack wip
> 
> f+. The infobar is not updated though (even if we change a value, so it's a
> more general problem).

I could do the same as I do for the deleteNode code from the context menu. Invalidate the breadcrumbs on editor save? We'd also want to update the infobar if id or classes change.

Comment 9

[Rob Campbell [:rc] (:robcee)]

Attachment: html attribute name editing

finished patch. Didn't make any changes to the tree panel code for this round. It's adequate. May refactor on a subsequent patch.

includes a test.

Comment 10

[Paul Rouget [:paul]]

Comment on attachment 604158 [details] [diff] [review]
html attribute name editing

Review of attachment 604158 [details] [diff] [review]:
-----------------------------------------------------------------

r+ , Please update the comment of the onTreeDblClick method to reflect its new behavior ("attribute value" -> "attribute name or value").

Comment 11

[Rob Campbell [:rc] (:robcee)]

Attachment: html attribute name editing v.2

done! thank you.

Comment 12

[Panos Astithas (he/him) [:past] (please ni?)]

https://hg.mozilla.org/integration/fx-team/rev/04f2053a28eb

Comment 13

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/04f2053a28eb