Closed Bug 1307941 Opened 8 years ago Closed 8 years ago

DOM nodes aren't being highlighted in the page on hover

Categories

(DevTools :: Console, enhancement, P2)

enhancement

Tracking

(firefox53 fixed)

VERIFIED FIXED
Firefox 53
Tracking Status
firefox53 --- fixed

People

(Reporter: linclark, Assigned: nchevobbe)

References

Details

(Whiteboard: new-console)

Attachments

(1 file)

Originally posted by:captainbrosset

see https://github.com/devtools-html/gecko-dev/issues/321

I searched github and bugzilla for this and couldn't find a relevant issue, so filing here:

STR:
- Open the console
- Enter `document.body`
- Hover over the output HTMLBodyElement

Expected: the <body> node should become highlighted in the page.
Actual: nothing happens.
Priority: -- → P2
Whiteboard: new-console
Depends on: 1307938
Assignee: nobody → chevobbe.nicolas
Status: NEW → ASSIGNED
Comment on attachment 8808777 [details]
Bug 1307941 - Add DOM nodes highlighter in new console frontend;

Brian, before going further I would like your opinion on this.
So what we do is have the higlight and unhighlight function in serviceContainer.
Then, when creating the Rep, we just pass a function that will return an object with event listeners (onMouseOver and onMouseOut), which call the highlight/unhighlight function. 
The function is then call in the Reps, and applied to the base element so the event listeners are on them.

The reason I wanted to experiment this way is that it makes little sense to have a wrapper element for the highlighter (like we do for the VariableView link), since we don't need some styling or anything else. I also think that since we need to keep the performance the best we can, not creating a wrapper element might help.

What do you think of this ?
Attachment #8808777 - Flags: feedback?(bgrinstead)
(In reply to Nicolas Chevobbe [:nchevobbe] from comment #2)
> Comment on attachment 8808777 [details]
> Bug 1307941 - Add DOM nodes highlighter in new console frontend; f=bgrins
> 
> Brian, before going further I would like your opinion on this.
> So what we do is have the higlight and unhighlight function in
> serviceContainer.
> Then, when creating the Rep, we just pass a function that will return an
> object with event listeners (onMouseOver and onMouseOut), which call the
> highlight/unhighlight function. 
> The function is then call in the Reps, and applied to the base element so
> the event listeners are on them.
> 
> The reason I wanted to experiment this way is that it makes little sense to
> have a wrapper element for the highlighter (like we do for the VariableView
> link), since we don't need some styling or anything else. I also think that
> since we need to keep the performance the best we can, not creating a
> wrapper element might help.
> 
> What do you think of this ?

Passing in props that get assigned to the appropriate child component seems OK to me.  But, will this approach work if the Reps HTML is cached (as in Bug 1308216)?

Also, rather than doing it as a mixin that is gets called and then automatically assigns all props, we could also just pass in onDOMNodeMouseOver and onDOMNodeMouseOut as props which get the object grip passed into it.  Might be a little simpler to follow (don't feel too strongly about it).
Comment on attachment 8808777 [details]
Bug 1307941 - Add DOM nodes highlighter in new console frontend;

https://reviewboard.mozilla.org/r/91518/#review91402

See coment 3
Attachment #8808777 - Flags: feedback?(bgrinstead) → feedback+
Comment on attachment 8808777 [details]
Bug 1307941 - Add DOM nodes highlighter in new console frontend;

https://reviewboard.mozilla.org/r/91518/#review92836

Works for me
Attachment #8808777 - Flags: review?(bgrinstead) → review+
Pushed by chevobbe.nicolas@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f6ae2c26b11a
Add DOM nodes highlighter in new console frontend; r=bgrins
https://hg.mozilla.org/mozilla-central/rev/f6ae2c26b11a
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Blocks: 1318060
I have reproduced this bug with Nightly 52.0a1 (2016-10-05) (64-bit) on Windows 7 , 64 Bit !

This bug's fix is verified with latest Nightly

Build   ID   20161130030206
User Agent   Mozilla/5.0 (WindowsNT 6.1; Win64; x64; rv:53.0) Gecko/20100101 Firefox/53.0
[bugday-20161130]
I have reproduced this bug with Nightly 52.0a1 (2016-10-05) (64-bit) on Ubuntu 16.10, 64 Bit !

This bug's fix is verified with latest Nightly

Build   ID   20161130030206

User Agent   Mozilla/5.0 (X11; Linux x86_64; rv:53.0) Gecko/20100101 Firefox/53.0

[bugday-20161130]
Setting this bug as Verified Fixed, according to comment 10 and comment 11.
Status: RESOLVED → VERIFIED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: