Closed
Bug 500365
Opened 16 years ago
Closed 16 years ago
removeProperty fails if ownerNode is null
Categories
(Core :: CSS Parsing and Computation, defect)
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: johnjbarton, Unassigned)
Details
Original report is http://code.google.com/p/fbug/issues/detail?id=1894
Issue 1894: CSS-edit removes declaration if page with specific javascript.
The report has a reproducible but complicated test case:
http://wesp.snt.utwente.nl/~alpha/firebug_test.htm
1. Install Firebug 1.5a6, http://getfirebug.com/releases/firebug/1.5X
2. load http://wesp.snt.utwente.nl/~alpha/firebug_test.htm
3. Open Firebug F12, enable Script and Net panel. Do *NOT* enable Console.
4. reload.
5. Inspect the page, click on C.S.V. Alpha
6. In the Style side panel of HTML panel, click on .componentheading's color value ("#666666"). An inline editor will appear.
7. Type in 'red'.
8a. Wait, (setTimeout)
8b. click outside Firebug, (onBlur)
8c. hit Return key (key).
You will pass through
http://mxr.mozilla.org/mozilla-central/source/layout/style/nsCSSStyleRule.cpp#1050
you will get an exception [nsIDOMCSSStyleDeclaration.removeProperty]" nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)",
and the edit will fail (most of the time).
After the fail you have to reload, the DOM is damaged.
Occurs on FF 3.5RC2 and FF 3.6 trunk from maybe 6 weeks ago.
At the point of failure, the declarations has
style.parentRule.parentStyleSheet.ownerNode == null.
I loaded the page a few times and looked at that declaration's ownerNode. With the Firebug console disabled, the ownerNode is usually null; with the Console enabled, the node is usually 'link'. (Right click on the declaration, Inspect in DOM Tab, then look for "get parentRule" etc.
So my guess is that the DOM load is failing in general to set ownerNode correctly. When Firebug's Console is enabled, then Firebug is injecting code right at the first JS compile on the page, shifting the workload and letting the correct value be set.
The test case is quite complicated but so far I have not been able to find a smaller case.
Reporter | ||
Comment 1•16 years ago
|
||
I have other reports of this problem; it does not happen on FF3.0; I guess it could be an indication of a problem outside of editing in Firebug.
Whiteboard: [firebug-p1]
Comment 2•16 years ago
|
||
Oh, this is going through the style panel? That's just using APIs known to be broken, assuming it works the way it does in DOM Inspector. In particular, editing styles that come from clone sheets using that panel will give you all sorts of weirdness. And in this case, I bet the difference is that you're picking up the clone of the preloaded sheet.
More generally, in that panel if you're looking at a rule you are NOT guaranteed that that rule is in the cssRules of the sheet it claims to be in. So DOM manipulation of the rules is just busted. We have existing bugs on this; probably filed on DOM Inspector.
Comment 3•16 years ago
|
||
To work around this, as soon as someone wants to use the style panel you need to walk through all sheets attached to the document (recursing into imported sheets) and get the first rule from the cssRules of each one....
Reporter | ||
Comment 4•16 years ago
|
||
(In reply to comment #2)
....
> So DOM manipulation of the rules is just busted. We have existing bugs on
> this; probably filed on DOM Inspector.
Something does not add up, since the CSS editor in Firebug is easily the most widely used Firebug feature. Surely we would have hit this before now.
Reporter | ||
Comment 5•16 years ago
|
||
(In reply to comment #3)
> To work around this, as soon as someone wants to use the style panel you need
> to walk through all sheets attached to the document (recursing into imported
> sheets) and get the first rule from the cssRules of each one....
You mean just access the first rule?
Comment 6•16 years ago
|
||
> Surely we would have hit this before now.
Cloned sheets are not that common in webpages. Until we did speculative sheet preloading you needed a web page that linked to a sheet twice. Or one that used a chrome stylesheet, of course...
> You mean just access the first rule?
Yes. Or any rule, really. The point is to force unique per-sheet rules. Cloned sheets share their rules, and the backdoor API the style panel uses to get at the rules doesn't force them to stop doing that...
Reporter | ||
Comment 7•16 years ago
|
||
Thanks Boris, at least in one quick test the problem is solved using code like:
if (!this.context.forcedUniqueStyleSheets)
{
if (this.context.loaded) // keep forcing until we are loaded
this.context.forcedUniqueStyleSheets = true;
iterateWindows(this.context.window, bind(function(win)
{
var doc = win.document;
var sheets = doc.styleSheets;
for(var i = 0; i < sheets.length; i++)
{
var rules = sheets[i].cssRules;
if (rules.length > 0)
var touch = rules[0];
if (touch)
FBTrace.sysout("css.show() touch "+typeof(touch)+" in "+win.location);
}
}, this));
}
Comment 8•16 years ago
|
||
For production use, you'd want to also walk into imported sheets, but that's the general idea, yes...
And now that I've had a chance to look for the original bug, marking duplicate. Note also bug 384327 (which is pretty much an exact duplicate).
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → DUPLICATE
Reporter | ||
Comment 9•16 years ago
|
||
Sometimes the line
var rules = sheets[i].cssRules;
gives an exception:
A parameter or an operation is not supported by the underlying object
NS_ERROR_DOM_INVALID_ACCESS_ERR
I wrapped it in a try block and ignored it.
Comment 10•16 years ago
|
||
You'll get that exception if you try to read a sheet before we've actually parsed it.
Reporter | ||
Comment 11•16 years ago
|
||
Touching the document.styleSheets as Boris suggests was implemented in Firebug 1.4 and works great, thanks Boris!
Resolution: DUPLICATE → WORKSFORME
Whiteboard: [firebug-p1]
Comment 12•16 years ago
|
||
Just to double-check, you did make sure to walk into imported sheets, right?
Reporter | ||
Comment 13•16 years ago
|
||
Well I have code like this:
for (var i = 0; i < sheet.cssRules.length; ++i)
{
var rule = sheet.cssRules[i];
if (rule instanceof CSSImportRule)
addSheet(rule.styleSheet);
}
But if you know of a test case I can verify it easily.
Comment 14•16 years ago
|
||
Yeah, that should do the trick.
You need to log in
before you can comment on or make changes to this bug.
Description
•