Closed Bug 729220 Opened 9 years ago Closed 9 years ago

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

Categories

(DevTools :: Inspector, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 13

People

(Reporter: rcampbell, Assigned: rcampbell)

References

Details

Attachments

(2 files, 1 obsolete file)

We currently only have editing of attribute values in the page inspector. We should allow editing of attribute names as well.
Assignee: nobody → rcampbell
Status: NEW → ASSIGNED
Depends on: 584407
Attached patch an egregious hack wip (obsolete) — Splinter Review
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.
Attachment #599280 - Flags: feedback?(paul)
Attachment #599280 - Flags: feedback?(dcamp)
asking dcamp for addl feedback.

hoping to get back to this patch this week.
(working on that today)
(ok, but be careful. it's ugly.)
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?
Attachment #599280 - Flags: feedback?(dcamp) → feedback+
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).
Attachment #599280 - Flags: feedback?(paul) → feedback+
(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.
(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.
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.
Attachment #599280 - Attachment is obsolete: true
Attachment #604158 - Flags: review?(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").
Attachment #604158 - Flags: review?(paul) → review+
done! thank you.
Attachment #604497 - Flags: review+
Attachment #604497 - Flags: review+
Whiteboard: [land-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/04f2053a28eb
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 13
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.