#1958 closed defect (wontfix)
dojo.html.insertCssText causes "Invalid procedure call or argument" in IE when styleSheet.disabled is true
Reported by: | Owned by: | alex | |
---|---|---|---|
Priority: | high | Milestone: | 0.9 |
Component: | General | Version: | 0.4.1rc1 |
Keywords: | stylesheet, disabled, IE, error | Cc: | |
Blocked By: | Blocking: |
Description
In dojo.html.style.js, in the function dojo.html.insertCssText, there is some special handling for IE for setting the css text on the style dom node:
if(style.styleSheet){ IE
style.styleSheet.cssText = cssStr;
}
However, if style.styleSheet.disabled is true, then you get the error "Invalid procedure call or argument" (both IE 6 and IE 7).
Why style.styleSheet.disabled is sometimes true I have no idea, it just is in one of my particular test cases. Unfortunately, I cannot reduce the test case to something small enough to post.
However, I have attached a workaround that solves the issue. I just do this instead:
if(style.styleSheet){ IE
if(!style.styleSheet.disabled){
style.styleSheet.cssText = cssStr;
}
}
Now I realize that without the reproducible test case, you might be a little hesitant to apply this patch (which I will attach shortly).
However, it stands to reason that even if you can't produce a test case where the style sheet ends up with disabled=true, if we just suppose that if it were somehow disabled, that attempting to modify it would throw some kind of error (and it does, trust me).
So why not just add the extra handling anyway as a precaution? It's not that many extra characters. :)
-aaron
Attachments (5)
Change History (23)
Changed 14 years ago by
Attachment: | 1958.patch added |
---|
comment:1 Changed 14 years ago by
Oh, BTW, in my case, I only saw this when at least all of the following were true (like I said, I can't quite pin it down):
- This was with a custom widget.
- Only happened when using a custom build (ie doesn't happen in dev mode) with intern strings.
- Only happened when I had a bunch of these widgets in the page.
- Never happened for widgets created from inline HTML. Happened only sometimes when another instance of the widget was created programmatically because of some user action in the UI. But it doesn't always happen under these circumstances.
-aaron
comment:2 Changed 14 years ago by
Owner: | changed from anonymous to alex |
---|---|
Status: | new → assigned |
comment:3 Changed 14 years ago by
Aaron: I'm a bit worried about this patch since it seems to imply that we'll just drop the style rules on the floor. That doesn't seem like a good idea. Given that we're inserting a brand new stylesheet for just this rule, I think I'm going to modify the patch to try again with a delay.
Regards
comment:4 Changed 14 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:5 Changed 14 years ago by
Sounds good. Out of curiosity, do you have any idea how the stylesheet object's disabled property would get set to true?
comment:6 Changed 14 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
It is still broken for me, even with the fix from the latest nightly build. It seems to break once you have too many widgets on a page. The page I'm having problems with has 33 tooltips and 23 dropdown menus. The tooltips work fine, but none of the dropdown menus do.
comment:7 Changed 14 years ago by
Josh Staiger ([email protected]…) and I were just discussing the issue on a thread on the mailing list.
See the thread "strange IE error".
Josh seems to have figured it out that IE can only handle up to 31 stylesheets associated with the document.
Further, that dojo must be adding duplicates causing this limit to be hit faster than necessary when the templateCssString property is used which is why you might not see the problem until you create your custom build with intern strings.
I think he has some kind of a patch and was going to check out this bug report and/or perhaps file a new one...
-aaron
comment:8 Changed 14 years ago by
Looks like it works if I use an uncompressed dojo build, but not compressed.
comment:9 Changed 14 years ago by
Patch from Josh Steiger (IBM). Basically, keep all CSS in a single <STYLE> element to get around the problem. Just keep appending text to the node. Does this cause significant performance problems? We could optimize by keeping a cache or looking for duplicate strings.
comment:10 Changed 14 years ago by
The reason why it works if you use the uncompressed dojo build is because even if you specific intern-strings the uncompressed build doesn't have strings interned.
So the culprit here is definitely intern-strings (I've tested with a compressed build without intern-strings and it works fine).
I have also discovered that when you exclude latency, intern-strings *kills* performance when you have many homogeneous widgets in a page (both FF and IE).
comment:11 Changed 14 years ago by
Milestone: | 0.4.1 → 0.5 |
---|
the patch is suspect and most of the problem is fixed in James' recent patch. Pushing to 0.5 until we get a patch that modifies individual CSS style rules instead of doing the += thing to redefine them all.
comment:13 Changed 14 years ago by
I have found a partial solution to this problem, although it would need quite a bit of work to make it tidy.
Although IE has a limit of 31 stylesheets, each stylesheet can have another 31 import stylesheets. And each import stylesheet can have more import stylesheets.
E.g. you can get around the 32 limit by creating dummy stylesheets at the 'head' level, and only adding stylesheets as import stylesheets:
var ss = style.styleSheet; ss.addImport(); ss.imports[ss.imports.length-1].cssText = cssStr;
The main downside is that you cannot return a reference to the style element (one style element may own many different imported stylesheets).
I will attached some crappy example files to show the fix in action.
Morris
Changed 14 years ago by
Attachment: | ie 30 style tags then die.html added |
---|
Test file showing use of import stylesheets
comment:14 Changed 14 years ago by
And a poorly written example of a function that allows you to add up to 60 stylesheets in IE:
function insertCssTextHack(cssStr){ if(!cssStr){ return; } if(!dojo.render.html.ie){ return dojo.html.insertCssText(cssStr); } var doc = document; var style = window.insertCssTextHackStyle; if (!style || style.styleSheet.imports.length == 30) { window.insertCssTextHackStyle = style = doc.createElement("style"); style.setAttribute("type", "text/css"); // IE is b0rken enough to require that we add the element to the doc // before changing it's properties var head = doc.getElementsByTagName("head")[0]; head.appendChild(style); } var ss = style.styleSheet; ss.addImport(); ss.imports[ss.imports.length-1].cssText = cssStr; return undefined; }
comment:15 Changed 14 years ago by
We ran into this problem recently. The most recent fix of creating a timeout if the stylesheet is disable didn't help. I found that Josh Steiger's patch worked well but I was concerned about the performance implications (per Alex's comments).
The patch I have in place right now is based on Josh Steiger's, but tests document.styleSheets.length first before deciding whether to create a new style sheet or append to an existing one. In the latter case, the most recent stylesheet is used (document.styleSheets[30]). Care is taken to limit this special processing to IE only. It's a compromise but a reasonable one.
To alleviate the potential for this problem, we have done an audit on our use of templateCssString in our home-grown widgets. There have been some benefits in doing this.
Just food for thought, but maybe there's an opportunity here to enhance the widget core code and provide another alternative to making css styles available to a widgets.
comment:16 Changed 14 years ago by
Just for those seeking solutions for v0.3.1:
There was a bug in ContentPane? that was fixed as part of a re-org in [4494] that meant <style> sections in ContentPane? were never removed when new content was loaded. Loading a few pages with <style> into ContentPane? would cause the error described in this ticket.
The patch (dojo0.3.1_ContentPaneCssUnloadFix.diff) fixes this against v0.3.1.
Changed 14 years ago by
Attachment: | dojo0.3.1_ContentPaneCssUnloadFix.diff added |
---|
Fix for ContentPane? styles not being unloaded in 0.3.1
comment:17 Changed 14 years ago by
Resolution: | → wontfix |
---|---|
Status: | reopened → closed |
Despite all the clever workarounds to this problem, marking won't fix, as we don't dynamically include CSS in Dijit anymore, and this dojo.html routine also goes away.
comment:18 Changed 13 years ago by
Morris here:
FYI Josh Steiger's solution works, my import stylesheets solution may cause IE6 memory leaks (detected using sIEve). I got a memory leak for *every* element in the page that had a filter expression (either from CSS rules or directly in element style of HTML).
The ingredients to cause the leak were:
- to use
addImport
(on blank stylesheet served in page, or on dynamically created stylesheet) - to use
imports[].cssText
(even if cssText set to nothing) - to use IE filter expressions (e.g.
filter: progid:DXImageTransform.Microsoft.AlphaImageLoader
orfilter: progid:DXImageTransform.Microsoft.Alpha
) - some other magic ingredient (leak didn't occur on static page, only on dynamic page)
I spent hours trying to work out what the magic ingredient was with no luck. If I removed the line setting cssText
, or if I removed all CSS filter:
expressions, then the leak went away.
patch for dojo.html.style.js