125082 - Can't inspect scrollbars or other native anonymous content

Status: RESOLVED FIXED

(Other Applications :: DOM Inspector, enhancement)

Description

[timeless]

There's a bug where scrollbars in mail for message pane have a context menu, but dom inspector wasn't helpful because it didn't list scrollbars.

Comment 1

[timeless]

hrm, i can inspect the threadpane scrollbar, but i can't inspect
<browser>/<iframe> scrollbars. fwiw there's no longer a context menu in mail
message pane.

Comment 2

[Alex Vincent [:WeirdAl]]

WFM Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.3b) Gecko/20030131

Settings for DOM Inspector:  
Left pane, DOM Nodes, browser element selected.
Right pane, JavaScript object, target, contentWindow, scrollbars

Comment 3

[Christopher Aillon (sabbatical, not receiving bugmail)]

Mass re-assigning bugs to [e-mail]

Comment 4

[Alex Vincent [:WeirdAl]]

per discussion with timeless:  This really isn't a bug caused by DOM Inspector.
 Note the last article in
http://weblogs.mozillazine.org/weirdal/archives/2004_05.html .

The problem is <xul:browser/>'s binding doesn't expose its scrollbars.  roc was
doing some work on our scrolling implementations around the time I wrote that
article (May, 2004).  I'm not sure if there are bugs filed which this could be
marked dependent on or not -- roc?

Comment 5

[Colby Russell :crussell (no longer Mozilla-ing)]

I'm changing the scope of this bug to include inspecting native anonymous content in general, since discriminating about scrollbars specifically is pretty arbitrary.  But this is to say nothing about whether that will ever happen.

Comment 6

[:Gijs (he/him)]

Attachment: Patch

It seems that both the domutils code and treewalker needed updating. This approach seems to work in my (limited) testing. Roc, can you see if this is more or less what you meant? :-)

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 736972 [details] [diff] [review]
Patch

Review of attachment 736972 [details] [diff] [review]:
-----------------------------------------------------------------

Yes!

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This is good, but any users of the inspector API inspecting anonymous content are going to get more then they bargained for. For example, DOMI users will see any scrollable element having scrollbars and a scrollcorner, which may confuse users. So I'd like to see a DOMI update that addresses that somehow, perhaps by putting that stuff in a different color. Also, any idea what other extensions use this API?

Possibly we should actually create new flags for inDeepTreeWalker to enable this.

Comment 9

[:Gijs (he/him)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #8)
> This is good, but any users of the inspector API inspecting anonymous
> content are going to get more then they bargained for. For example, DOMI
> users will see any scrollable element having scrollbars and a scrollcorner,
> which may confuse users. So I'd like to see a DOMI update that addresses
> that somehow, perhaps by putting that stuff in a different color. Also, any
> idea what other extensions use this API?

So I checked all the add-ons on AMO. This is all I found:

- various DOM Inspector copies
- https://addons.mozilla.org/en-us/thunderbird/addon/subswitch/ (code inspection would suggest it should be OK with this change, it just looks for a specific node for some reason, and its code is OK with random other stuff being thrown at it, AFAICT)

> 
> Possibly we should actually create new flags for inDeepTreeWalker to enable
> this.

I would say that if in DOMI you enable anon content, this is what you want, and you don't need an extra switch / coloring. Could we live with this patch and see if people are desperate for more differentiation in terms of coloring and/or prefs?

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

ok

Comment 11

[:Gijs (he/him)]

(In reply to :Gijs Kruitbosch from comment #6)
> Created attachment 736972 [details] [diff] [review]
> Patch
> 
> It seems that both the domutils code and treewalker needed updating. This
> approach seems to work in my (limited) testing. Roc, can you see if this is
> more or less what you meant? :-)

Sadly, this failed a mochitest on try ( https://tbpl.mozilla.org/?tree=Try&rev=9df10c6a0f36 ). Will look into why later today...

Comment 12

[:Gijs (he/him)]

Attachment: Patch that doesn't crash

I'm not sure I can carry over review, so I'm not doing so. However, it's pretty much the same patch except I've reorganized the inDeepTreeWalker PushNode code to be more similar to the inDOMUtils code, meaning it no longer crashes in case GetChildren returns null. This makes the previously-crashy test pass on my local box, but I've fired off another try run just to be sure: https://tbpl.mozilla.org/?tree=Try&rev=ba1c7148bc36 (the test is in M5).

Comment 13

[:Gijs (he/him)]

(In reply to :Gijs Kruitbosch from comment #12)
> Created attachment 737295 [details] [diff] [review]
> Patch that doesn't crash
> 
> I'm not sure I can carry over review, so I'm not doing so. However, it's
> pretty much the same patch except I've reorganized the inDeepTreeWalker
> PushNode code to be more similar to the inDOMUtils code, meaning it no
> longer crashes in case GetChildren returns null. This makes the
> previously-crashy test pass on my local box, but I've fired off another try
> run just to be sure: https://tbpl.mozilla.org/?tree=Try&rev=ba1c7148bc36
> (the test is in M5).

Green run + review = checked in! :-)

https://hg.mozilla.org/integration/mozilla-inbound/rev/d31055321344

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d31055321344