If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

[markup view] Empty tags should not be expandable in the tree

RESOLVED FIXED in Firefox 26

Status

()

Firefox
Developer Tools: Inspector
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: bgrins, Assigned: pbro)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 26
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
If you click on a script or link tag with no content inside of the markup view, there is no twisty arrow by it to expand, but if you press the right arrow it will expand anyway.  It should not do this.
(Reporter)

Updated

4 years ago
Blocks: 831711
(Reporter)

Updated

4 years ago
Depends on: 855523

Comment 1

4 years ago
Created attachment 799776 [details] [diff] [review]
patch: only expand containers that have children

This short patch seems to fix it. Do you think this is right?
Assignee: nobody → mjh563
Status: NEW → ASSIGNED
Attachment #799776 - Flags: review?(paul)

Updated

4 years ago
Attachment #799776 - Flags: review?(paul) → feedback?(bgrinstead)
(Reporter)

Comment 2

4 years ago
Comment on attachment 799776 [details] [diff] [review]
patch: only expand containers that have children

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

This looks good to me, but pbrosset should have a look at it, as he may have some related work in Bug 855523
Attachment #799776 - Flags: feedback?(pbrosset)
Attachment #799776 - Flags: feedback?(bgrinstead)
Attachment #799776 - Flags: feedback+
(Assignee)

Comment 3

4 years ago
I think it makes more sense to add this logic at the `_onKeyDown` level:

  ...
  case Ci.nsIDOMKeyEvent.DOM_VK_RIGHT:
    if (!this._selectedContainer.expanded && this._selectedContainer.hasChildren) {
      this._expandContainer(this._selectedContainer);
    }
  ...

I've had cases where the promise returned by `this._updateChildren(aContainer, true)` wasn't waiting for children to be ready, and therefore the `if (this.hasChildren)` in your code wouldn't expand on these nodes.

For information, the markup-view will change quite a lot when bug 855523 lands, so we might want to hold back a bit for this one.

Comment 4

4 years ago
OK, so it seems as though I was a bit premature with the patch. :)

Unassigning myself for now.
Assignee: mjh563 → nobody
Status: ASSIGNED → NEW
(Assignee)

Updated

4 years ago
Assignee: nobody → pbrosset
(Assignee)

Comment 5

4 years ago
I confirm that this bug has been fixed via bug 855523 which just landed.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
(Assignee)

Updated

4 years ago
Attachment #799776 - Flags: feedback?(pbrosset)
You need to log in before you can comment on or make changes to this bug.