1436413 - layout/inspector/inDOMView.cpp seems unused

Status: RESOLVED FIXED

(Core :: Layout, enhancement, P3)

Comment 1

[Marco Castelluccio [:marco]]

Attachment: Remove inDOMView as it is no longer used

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8949072 [details] [diff] [review]
Remove inDOMView as it is no longer used

It's used by comm-central.  Have you coordinated with them?

Comment 3

[Marco Castelluccio [:marco]]

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #2)
> It's used by comm-central.  Have you coordinated with them?

Not yet.

Philipp, Frank-Rainer, Stefan, Ian, what do you think of this?

Comment 4

[Philipp Kewisch [:Fallen][📱📆]]

IIUC this is only being used by the DOM Inspector. Given comm makes use of devtools now, it is unclear to me if there is a need to support DOMi any more. As per module owner page, Shawn and Neil own the component, adding ni? for them (although I can't find bugmail for Shawn).

If it were up to me I would go ahead and remove extensions/inspector from m-c, along with fixing this bug.

Comment 5

[Frank-Rainer Grahl (:frg)]

Neil is curently no longer active. We currently do not build DOMi. It is currently broken and only working till 58 because of other removals but we hope to fix it up again when we find some time. Not sure if this can be done when inDOMView is removed. It has its own repo but can be removed from mozilla\extensions. We already creates suite\extensions for this.

Comment 6

[Boris Zbarsky [:bzbarsky]]

If inDOMView is removed, the DOM tree view in inspector will be completely broken as things stand.

I think the main options are:

1)  Remove inDOMView, kill inspector.
2)  Move inDOMView to c-c, make sure to build it as part of libxul.  Maintenance pain
    as we do more XPCOM removals...  It uses a bunch of nsIDOM* bits and is non-trivial to update.
3)  Try to rewrite inDOMView in JS.  Expose APIs as needed to make that possible.
    If this is done, this can live in c-c.
4)  Rewrite the DOM view in inspector to some mechanism other than a treeview.
5)  Leave everything as it is.

Options 1, 3, 4 are great from the point of view of removing nsIDOM* stuff.  Option 2 is great from that pov for m-c, but sucks from the c-c side, I suspect.  Option 4 is awesome from the "remove scripted treeviews" point of view... 

I don't know that I have strong opinions here, really.

Comment 7

[Frank-Rainer Grahl (:frg)]

I spoke with some Thunderbird devs yesterday. One is still using DOMi but the others are already use devtools. It seems SeaMonkey has a bug and devtools in it can't inspect chrome documents. I wasn't aware of this and in view of this it is imho better to fix this and retire DOMi. That is what I use DOMi most for.

Comment 8

[Boris Zbarsky [:bzbarsky]]

So just to be clear, are we explicitly killing off DOMi?  Because if so, there may be a whole bunch of stuff in addition to inDOMView that we can remove.

Comment 9

[Jet Villegas (inactive)]

[ Triage 2017/02/20: P3 ]

Comment 10

[Marco Castelluccio [:marco]]

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #8)
> So just to be clear, are we explicitly killing off DOMi?  Because if so,
> there may be a whole bunch of stuff in addition to inDOMView that we can
> remove.

I've filed bug 1439705 for that.

Comment 11

[[e-mail]]

FYI Ways in which I found DOM Inspector better than devtools inspector:
1. Faster
2. Able to inspect menus (devtools stops menus from opening when its picker is active)
3. Position information in box model (or am I missing something?)
4. Computed style shows everything
5. XBL support

Comment 12

[Philipp Kewisch [:Fallen][📱📆]]

Neil, nice to hear from you! As technically you are still a peer for the module, maybe you could give your input on if it is ok to remove in bug 1439705. On your points:

(In reply to [e-mail] from comment #11)
> FYI Ways in which I found DOM Inspector better than devtools inspector:
> 1. Faster
Can't argue that :D

> 2. Able to inspect menus (devtools stops menus from opening when its picker
> is active)
I think this should be possible, I believe some pref was set to keep popups like menus from closing if devtools is open. I'm not quite sure how it works though. Maybe worth filing a bug.


> 3. Position information in box model (or am I missing something?)
bug 1113483 (looks like it was removed again though, might be worth commenting there)

> 4. Computed style shows everything
If you enable "browser styles" it shows more, if anything is missing I'd suggest filing a bug

> 5. XBL support
XBL is supported, at least I can look into anonymous content in our developer tools. If anything specific is missing it will probably be WONTFIXed since they are trying to get rid of XBL :D (unless it translates to shadow DOM and web components)

Comment 13

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

On this topic, there is an existing meta-bug (bug 1090423) that collects various areas where Browser Toolbox still needs some work compared to DOM Inspector.

Comment 14

[Sylvestre Ledru [:Sylvestre]]

What about moving the code of inDOMView into the comm-central source tree and remove it from m-c?
Or we could just stop building it in Fx source tree.

Comment 15

[Philipp Kewisch [:Fallen][📱📆]]

For this bug I think you should go ahead and move forward with the removal from m-c, if for some reason we need it in c-c then we can resurrect it.

Comment 16

[Boris Zbarsky [:bzbarsky]]

> What about moving the code of inDOMView into the comm-central source tree and remove it from m-c?

Sylvestre, see comment 6.

In practice, I expect that option 2 from comment 6 is not viable.  I have no problem going with option 1 and reviewing this patch; I will do that tomorrow.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8949072 [details] [diff] [review]
Remove inDOMView as it is no longer used

r=me.  Sorry for the lag; wanted to make sure I understood the general future of this code.

On a personal level, so glad to see it gone... ;)

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/322f27869c71
Remove inDOMView as it is no longer used. r=bz

Comment 19

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/322f27869c71

Comment 20

[Marco Castelluccio [:marco]]

The removal landed, clearing the needinfo.