>>> My Info: Win7_64, Nightly 46, 32bit, ID 20160101030330 STR: 1. Open attached "testcase 1" or this url: > data:text/html,<object></object><embed></embed> 2. Open Inspector 3. Click the dropmarker next to <embed> element in markup-view 4. Click the dropmarker next to <object> element in markup-view Result: Both elements are expanded in markup-view Expectations: This is inconsistency (due to bug 892935): there should be no dropmarkers at all. Note (IMO): Looking at all mistakes in implementing bug 892935 and all usability issues it has caused, I believe that the best way was NOT to forbid expanding nodes, but, conversely, to improve the way nodes (including text nodes) are expanded.

Thanks for filing. This is indeed pretty weird. After a quick investigation, I can see that these nodes are flagged as having 1 child element (with the node selected, inspector.selection.nodeFront.numChildren == 1), but then, trying to get the child element returns nothing: walker.children(inspector.selection.nodeFront.numChildren).then(({nodes}) => nodes.length == 0); Investigation should take place in /devtools/server/actors/inspector.js especially in the WalkerActor's children method and the way the numChildren property is set in NodeActor. Somehow, there's a mismatch between the two.

Hi Patrick, I trace the code and found the problem here. https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/inspector.js#352 The truthy value from `rawNode.getSVGDocument` let the numChildren always equal to 1. And thus will not entered the next if statement which giving the correct numChildren value through `this.walker.children(this).nodes.length`. It looks like we are having several solutions here, like avoiding the assignment from `numChildren = 1;` or let it consult with the walker. Which do you think is the proper solution and won't break the design here? Thanks!

Thanks for the investigation Joseph. You're right `rawNode.getSVGDocument` is truthy here and therefore sets `numChildren` to 1. I think the right might be to actually execute `getSVGDocument` instead of just checking that it exists. We test the existence of `rawNode.getSVGDocument` to see if the node embeds SVG, but we don't execute that function. And it turns out that for <object> and <embed> this function exists, but may actually return `null`. I believe we might be able to fix the issue by changing the if to: let hasContentDocument = rawNode.contentDocument; let hasSVGDocument = rawNode.getSVGDocument && rawNode.getSVGDocument(); if (numChildren === 0 && (hasContentDocument || hasSVGDocument)) {

Thanks for your advice, I'll push this patch to try server later.

Comment on attachment 8767840 [details] [diff] [review] 0001-Bug-1236283-object-and-embed-nodes-in-markup-view-ca.patch Review of attachment 8767840 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me, thanks. Can you update the comment message however? The commit message should not state the problem (or bug title) but instead explain what you changed. Plus, it should contain the reviewer's nick. Something like: Bug 1236283 - Don't mistake <embed> and <object> as having 1 child node when they don't contain anything; r=pbro

Sorry about that, I've update the patch. Thanks for your help!

Comment on attachment 8767865 [details] [diff] [review] 0001-Bug-1236283-Don-t-mistake-embed-and-object-as-having.patch Review of attachment 8767865 [details] [diff] [review]: ----------------------------------------------------------------- Thanks! R+ from me, please land this as soon as you get a green TRY.

All green on try. https://treeherder.mozilla.org/#/jobs?repo=try&revision=52490d08acfe

Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/fx-team/rev/3468f546b085 Don't mistake <embed> and <object> as having 1 child node when they don't contain anything; r=pbro

sorry had to back this out in https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=b9cc2dcbf4363c8049183110fe79e026fae87ecb since one of this 2 changes seems have caused https://treeherder.mozilla.org/logviewer.html#?job_id=10330926&repo=fx-team can you take a look since i'm not sure which of this 2 changes caused this regressions

Backout by cbook@mozilla.com: https://hg.mozilla.org/integration/fx-team/rev/9ef4fa72ea0b Backed out changeset 3468f546b085

The regression was mine from bug 1171614, so this is fine to go back in.

Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/fx-team/rev/9f227558d11f Don't mistake <embed> and <object> as having 1 child node when they don't contain anything. r=pbro

Fixed on: Win7_64, Nightly 50, 32bit, ID 20160714030208 (2016-07-14)