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

RESOLVED FIXED

Status

Other Applications
DOM Inspector
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: crussell, Assigned: crussell)

Tracking

(Blocks: 1 bug)

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

It starts and never finishes a tree box object update batch.
Created attachment 498535 [details] [diff] [review]
take one dip and end it
Assignee: nobody → Sevenspade
Status: NEW → ASSIGNED
Attachment #498535 - Flags: review?(neil)

Comment 2

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

What exception is this trying to catch?

Comment 3

7 years ago
I notice that none of the other relevant methods have a try/catch either.
(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

7 years ago
(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
Created attachment 499370 [details] [diff] [review]
use try/finally
Attachment #498535 - Attachment is obsolete: true
Attachment #499370 - Flags: review?(neil)
Attachment #498535 - Flags: review?(neil)

Comment 7

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

Finally!

[Sorry for the http://tvtropes.org/pmwiki/pmwiki.php/Main/IncrediblyLamePun .]
Attachment #499370 - Flags: review?(neil) → review+
Pushed: http://hg.mozilla.org/dom-inspector/rev/85e3911ecb47
Blocks: 592530
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.