Closed Bug 1202254 Opened 9 years ago Closed 7 years ago

Node in markup-view can be expanded if it contains only text " "

Categories

(DevTools :: Inspector, defect, P2)

Product:
DevTools
Component:
Inspector
Type:
defect

Tracking

(firefox43 affected)

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox43 --- affected

People

(Reporter: arni2033, Unassigned)

References

(Blocks 1 open bug,
URL
)

Details

(Whiteboard: [btpp-fix-later][polish-backlog][difficulty=medium])

1. Open the followind "data:" URL (or click URL in form above) data:text/html,<body>&nbsp;</body> 2. Open devtools->Inspector 3. Click dropmarker near <body> element Result: <body> can be expanded Expectations: Due to bug 892935 this shouldn't happen. Text should be included inline, just like on page data:text/html,<body>L&nbsp;</body> Note: the issue may be more general than just "&nbsp;"
This is because we filter out empty whitespace nodes [0]. There's a bug here where empty whitespace nodes *are* being returned in numChildren [1], so the UI adds an expander arrow because numChildren is 1. A couple of options here: a) We could be smarter about not filtering out whitespace text nodes in standardTreeWalkerFilter if it's the only child node b) We could do additional filtering in numChildren so that in this test case <body> wouldn't be expandable. Actually we could probably directly call the Walker's filter() function from numChildren to get a more accurate count without needing to do a full call to walker.children(). Patrick, what do you think the desired behavior is here? [0]: https://dxr.mozilla.org/mozilla-central/rev/5fe9ed3edd6811a662d40d05e37b0d66e9520d82/toolkit/devtools/server/actors/inspector.js#3867-3871 [1]: https://dxr.mozilla.org/mozilla-central/rev/5fe9ed3edd6811a662d40d05e37b0d66e9520d82/toolkit/devtools/server/actors/inspector.js#309
Flags: needinfo?(pbrosset)
(In reply to Brian Grinstead [:bgrins] from comment #1) > This is because we filter out empty whitespace nodes [0]. There's a bug > here where empty whitespace nodes *are* being returned in numChildren [1], > so the UI adds an expander arrow because numChildren is 1. > > A couple of options here: > a) We could be smarter about not filtering out whitespace text nodes in > standardTreeWalkerFilter if it's the only child node > b) We could do additional filtering in numChildren so that in this test case > <body> wouldn't be expandable. Actually we could probably directly call the > Walker's filter() function from numChildren to get a more accurate count > without needing to do a full call to walker.children(). > > Patrick, what do you think the desired behavior is here? b) sounds like what we should be doing. Conditionally filtering whitespace nodes if they're the only child node (a) would work but will require somewhat complex code and comments to explain why we do it.
Flags: needinfo?(pbrosset)
Hm, I'd like to add that in my example I can't doubleclick the inline text to start editing... So nbsp should be displayed like a regular " ". Actually, inline text could have min-width for easy clicking
Whiteboard: [polish-backlog]
Whiteboard: [polish-backlog] → [polish-backlog][difficulty=medium]
Blocks: 1228689
Inspector bug triage (filter on CLIMBING SHOES).
Priority: -- → P2
Whiteboard: [polish-backlog][difficulty=medium] → [btpp-fix-later][polish-backlog][difficulty=medium]
The STRs from the summary are now fixed.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.