211537 - Dom Inspector not working properly-doesn't display full tree of nodes

Status: RESOLVED FIXED

(Other Applications :: DOM Inspector, defect)

Description

[Charles Fenwick]

In today's build, I tried examining a site in the DOM inspector and was unable
to do so.

To reproduce: Bring up the DOM inspector and type a website into the address bar
and press enter (mozilla.org for instance)

After the page loads, notice in the left hand pane there's only two items
visible #document and html.
There's nothing to expand.

Compare to a fully functioning DOM Inspector. When you bring up the page, you
see in the left pane #document, html, html.  Clicking on the lower html expands
the tree to head, text, body, etc.

This works properly in 062808 build (the most recently nightly I have besides
this one).

Comment 1

[Alex Vincent [:WeirdAl]]

I've seen the bug Mr. Fenwick reports here; the reason dbaron and bz are cc'd
are because they've authored the only two checkins to extensions/inspector/*.*
in the last seven days.

DOM Inspector is totally unuseable with this regression.

Comment 2

[Charles Fenwick]

*** Bug 211539 has been marked as a duplicate of this bug. ***

Comment 3

[Boris Zbarsky [:bzbarsky]]

So... The main tree renders fine for me, but I see some problems with attributes
not being shown (in fact, spent a few hours today trying to figure out which of
my local changes caused that).

Given the hint that it's present in nightlies, I've found the culprit for my
attribute problem -- the checkin for bug 208093.  I seem to recall seeing some
tree assertions in Inspector even in my debug build (which did not have the
patch for bug 208903 in it at the time...).

Neil, any ideas as to what Inspector is doing wrong with its tree magic?

Comment 4

[[e-mail]]

Attachment: Proposed patch

Comment 5

[[e-mail]]

Comment on attachment 126991 [details]
Proposed patch

Begin/End Update Batch are new methods that Jan recently added to make it easy
to rebuild a tree.

Comment 6

[Boris Zbarsky [:bzbarsky]]

So... I can't sr this unless I have some idea of why this works?  Why is calling
beginUpdateBatch/endUpdateBatch supposed to work while invalidate() does not? 
Calling those without doing anything in between seems a bit fishy to me in any
case...

Also, the JS changes look wrong -- what's beginBatchUpdate? 

Comment 7

[[e-mail]]

Comment on attachment 126991 [details]
Proposed patch

Ah, I'd forgotton to check that code path :-/

Comment 8

[[e-mail]]

Attachment: Try again :-[

The point of invalidate() only requests that the tree repaint itself.
If you change the number of rows in the tree, then you could call
rowCountChanged() but that requires you to a) add or remove all the rows at the
same point b) know each time you call rowCountChanged() what the change and new
number of rows is.
This was inconvenient when rebuilding from an RDF data source (bookmarks in
particular) so Jan added begin/endUpdateBatch methods to suppress checks and
allow you to muck about generally when making extensive changes to the tree.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 127011 [details] [diff] [review]
Try again :-[

How about clearly documenting that in the interface or something useful like
that?

So what this patch does is to ask the tree to completely rebuild itself on any
sort of random changes, huh?  I suppose that works, though it may be overkill
for the removeProperty case... (for setProperty it may actually be needed).

Also, shouldn't the update batch begin _before_ we make the actual changes to
the style rule?  I guess it doesn't matter since the rule won't be affecting
the tree, but that would make a lot more sense to me from a style point of
view....

sr=me with that change.

Comment 10

[[e-mail]]

Attachment: Possible nsITreeBoxObject.idl

Comment 11

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 127011 [details] [diff] [review]
Try again :-[

r=caillon with bz's requested changes.

Comment 12

[[e-mail]]

Fix checked in.

Comment 13

[Christopher Aillon (sabbatical, not receiving bugmail)]

Reopened for two reasons:

1. The assignee of this bug is incorrect.  Please fix that.
2. This checkin modified EVERY LINE in 2 files and was backed out by me.

Comment 14

[Christopher Aillon (sabbatical, not receiving bugmail)]

Yes, it was turning ports red and you were nowhere to be found.  I am going to
try and get the rcs files fixed on Monday when leaf is around, so we can get a
reasonable blame history again.  In the meantime, I suggest not re-landing this
patch until then.

Comment 15

[Brendan Eich [:brendan]]

Don't mean to interfere (CVS history preservation is a good thing, enough to
warrant queueing up for leaf to do the deed), but turning ports red is not
enough cause for a back-out, strictly speaking.  I should know, I've burned some
ports lately and picked up the pieces a day or two later.

/be

Comment 16

[Alex Vincent [:WeirdAl]]

We probably don't want to ship 1.5a with a broken Inspector.

Comment 17

[[e-mail]]

Line endings problem :-[