Open Bug 1324254 Opened 7 years ago Updated 2 years ago

Inspecting an element selects the <body> element before the right element is selected inside the node view

Categories

(DevTools :: Inspector, defect, P2)

Product:
DevTools
Component:
Inspector
Version:
53 Branch
Type:
defect

Tracking

(firefox53 affected)

Tracking Flags:
Tracking Status
firefox53 --- affected

People

(Reporter: sebo, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: parity-chrome, Whiteboard: [dt-q]) 

When an element is inspected within the page, the <body> element is selected first before the actual element is selected. This causes a poor UX.
To reproduce the issue right-click this text and choose 'Inspect Element' from the context menu.

Sebastian
This is because devtools/client/inspector/markup/markup.js::_onNewSelection() calls showNode().

showNode() then expands the node's parents one by one and then centers the selected node in the tree. As each parent node is selected to expand the branch of the tree it is marked as selected and the sidebar tools are updated.

I don't believe that the nodes are lazy loaded and expanding them certainly doesn't mean that we should show each parent node in the side panels... this should only happen when the final target node is reached.

An obvious fix would be to have an isSelecting flag so we know whether we should be updating the sidebar tools.
(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #1)
> showNode() then expands the node's parents one by one and then centers the
> selected node in the tree. As each parent node is selected to expand the
> branch of the tree it is marked as selected and the sidebar tools are
> updated.

Note that the speed of this process regressed lately, i.e. before it took a few milliseconds, now it sometimes takes up to a second or even longer (on my cheap laptop). I couldn't find a regression window for this yet, though.

> I don't believe that the nodes are lazy loaded and expanding them certainly
> doesn't mean that we should show each parent node in the side panels... this
> should only happen when the final target node is reached.

Absolutely. Note that the speed of this process regressed lately, i.e. before it took a few milliseconds, now it sometimes takes several seconds (on my feeble laptop)! I couldn't find a regression window for this yet, though. Did the logic related to this change lately?

Sebastian

PS: FYI, several former Firebug users already complained about this. Because the inspector is one of the most commonly used features, that bug should get high priority.
I'm getting several complaints about this in https://groups.google.com/d/topic/firebug/PiCXxzDdsO8/discussion and other places.
And I was lucky to be able to show the issue to Mike live at the All Hands.

As I wrote in the previous comment, this is an essential DevTools feature. And having it not work as expected is a real UX pain. So I believe this should be fixed asap.
Patrick, could you please prioritize this bug?

Sebastian
Flags: needinfo?(pbrosset)
Inspector bug triage (filter on CLIMBING SHOES).
Blocks: top-inspector-bugs
Flags: needinfo?(pbrosset)
Priority: -- → P2
Weird, I can't reproduce this problem (Win10), it just works for me.

(btw. there is another report related to Firebug's inspect element - bug 1335853)

Honza
Maybe your machine is fast enough that the effect is too short to see it.
On my slow laptop it is reproducible with 51.0.1 as well as with 54.0a1. And I read several forum posts lately saying the same, last one yesterday[1].

Sebastian

[1] https://groups.google.com/d/topic/firebug/c_--f-8p9mE/discussion
I'm using FireFox 51.0.1 (64-bit) and FireBug 2.0.18 and have the same problem. Inspect Element selects the body tag instead of the actual tag.
I am a web developer. I've developed websites in Firefox since 2007. So it's 10 years now. I always make them in Firefox and then test in Chrome, IE, Opera and others and make changes. 
Firefox is also my primary browser. 

Firebug is now discontinued and Firefox's "Insect Element" always selects body. So for me Firefox is completely unusable to develop websites.

This issue was reported 2 MONTHS ago and is still not fixed. I am really tired of waiting. And I believe there are thousands of web developers who need this feature right NOW.
For me your message is clear- all of us have to switch to Chrome because "**** web developers".
Comment 7 and comment 8 seem to be about a different problem. Apparently the <body> node *stays* selected. Whereas this bug is about the <body> node being selected first, and then the right node being selected right after that. Sebastian is reporting that the inspector is being slower than it used to be, and so this 2-steps selection process is more visible than before.
While the last 2 comments indicate that the right node doesn't even get selected at all.

I am wondering if what comment 7 and comment 8 reports is in fact bug 1335853.

Do you have 2 "inspect element" menu items in the firefox context menu? If yes, which one did you choose?
Flags: needinfo?(anjanesh)
That's right, the <body> node stays selected. The right node doesn't get selected at all.
Is there a different bug report on this behaviour ?

I have two inspect element menu items in FireFox context menu. One is "Inspect Element (Q)" and the other is "Inspect Element with FireBug". I'm using the latter. The former works.

FireFox 51.0.1 (64-bit) + Firebug 2.0.18 on Ubuntu 14.04
(In reply to Anjanesh Lekshminarayanan from comment #10)
> I have two inspect element menu items in FireFox context menu. One is
> "Inspect Element (Q)" and the other is "Inspect Element with FireBug". I'm
> using the latter. The former works.
Thank you for confirming. This is indeed another bug as I thought: bug 1335853, which we will actually fix in the Firebug addon, so the issue is tracked here: https://github.com/firebug/firebug/issues/8077
Flags: needinfo?(anjanesh)
Thank you, Patrick, for clarifying the difference! There were already several users complaining about that and I assumed they use the 'Inspect Element' option.

Both issues still need to be fixed quite urgently, IMO.

Sebastian
Sebastian, I'm trying to reproduce this issue but without luck. I also tried to run the Nightly build on a Linux VM so that would be slow enough, but still I don't see anything. Before digging in the code, since we change a lot recently in markup view and inspector, could you confirm that with the latest build you still have this bug? Is there any page – maybe it depends also by the page's complexity – that you can recommend to test it? Thanks!
Flags: needinfo?(sebastianzartner)
Assignee: nobody → zer0
Status: NEW → ASSIGNED
I can still reproduce the issue in Nightly 55.0a1 (2017-03-24). The page doesn't matter, but more complex pages make the issue more obvious.

Simple test case:
1. Go to https://www.mozilla.org/en-US/
2. Right-click on the Mozilla logo and choose 'Inspect Element' from the context menu

=> You can see that the toolbox opens, then the main panel, then the side panel, then the node view is filled with <body> being selected, *then the actually inspected element is selected* and the Rules panel is filled to show its CSS rules.

On faster PCs this happens within a second or two, so you won't notice that <body> is selected first. Though on my slow laptop these steps can take up to twenty seconds or more!

Sebastian
Flags: needinfo?(sebastianzartner)
Matteo, I forgot to explicitly mention one important point: This only happens when the DevTools are initially closed. When they are open when you call 'Inspect Element', the <body> element is *not* selected first.
Does this make a difference for you?

Sebastian
Flags: needinfo?(zer0)
Thanks Sebastian, I misunderstood initially the issue; I'm able to reproduce every time.

So, I believe I understand why is happening, and I know ideally how to fix it, but I'm not sure how to proceed yet.

Basically when we inspect a node from the context menu, we're calling this method:

http://searchfox.org/mozilla-central/source/browser/base/content/nsContextMenu.js#628-633

That invokes:

http://searchfox.org/mozilla-central/source/devtools/client/framework/devtools-browser.js#310-343

That basically creates the toolbox, the inspector, and set the node to inspect:

    inspector.selection.setNodeFront(nodeFront, "browser-context-menu");

The problem here, is that when an inspector is created, it set a "default node":

http://searchfox.org/mozilla-central/source/devtools/client/inspector/inspector.js#142

And that's why the <body> is selected first, and then the node from the contextual menu inspected after.

I don't know much of this code, so I'm not sure yet what is the best course of action: the issue here is that as soon as we create the inspector instance, that we need to set the node to inspect, a "default node" was already selected. We should be able, basically, to pass the information about the node we want to inspect *before* the inspector is created, so that we can take in account this data when we get the default node for the selection.
Flags: needinfo?(zer0)
Blocks: 1378964
Assignee: zer0 → nobody
Status: ASSIGNED → NEW
Product: Firefox → DevTools
Seeing this too in 64b10 (Beta Channel and Developer Edition).

Doesn't happen on every site. But consistently on some. Closing Dev Tools and re-opening them helps.
See Also: → 1589320
No longer blocks: top-inspector-bugs
Keywords: parity-chrome
Whiteboard: [dt-q]
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.