620149 - Unable to select or scroll properties tree in CSS Rules viewer after undoing a deletion

Status: RESOLVED FIXED

(Other Applications :: DOM Inspector, defect)

Description

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

It starts and never finishes a tree box object update batch.

Comment 1

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

Attachment: take one dip and end it

Comment 2

[[e-mail]]

Comment on attachment 498535 [details] [diff] [review]
take one dip and end it

What exception is this trying to catch?

Comment 3

[[e-mail]]

I notice that none of the other relevant methods have a try/catch either.

Comment 4

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

(In reply to comment #2)
> Comment on attachment 498535 [details] [diff] [review]
> take one dip and end it
> 
> What exception is this trying to catch?

Warning: Cannot specify value for internal property.  Error in parsing value for 'padding-left-value'.  Declaration dropped.
Source File: chrome://global/skin/toolbar.css
Line: 0

This occurs when you try to undelete that property.  removeProperty works fine, as well as setProperty with an empty value, but any significant value throws.

(In reply to comment #3)
> I notice that none of the other relevant methods have a try/catch either.

cmdEditInsert does.  Note that it's the only other transaction that uses begin-/endUpdateBatch together with setProperty.  That the others lack a try/catch is fine because in the event they throw, the error is reported to the console, and the tree remains usable.

Comment 5

[[e-mail]]

(In reply to comment #4)
> (In reply to comment #2)
> > (From update of attachment 498535 [details] [diff] [review])
> > What exception is this trying to catch?
> Warning:
[That's not an exception, although the standard says it should be...]

> (In reply to comment #3)
> > I notice that none of the other relevant methods have a try/catch either.
> cmdEditInsert does.
The reason I didn't notice was that I searched for catch, and cmdEditInsert actually uses try/finally.

So I would like one of three options:
1. Just patch this case but use finally for consistency
2. Patch all callers of setProperty to use try/finally
3. Patch all callers of setProperty to use try/catch

Comment 6

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

Attachment: use try/finally

Comment 7

[[e-mail]]

Comment on attachment 499370 [details] [diff] [review]
use try/finally

Finally!

[Sorry for the http://tvtropes.org/pmwiki/pmwiki.php/Main/IncrediblyLamePun .]

Comment 8

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

Pushed: http://hg.mozilla.org/dom-inspector/rev/85e3911ecb47