Closed GoogleCodeExporter closed 8 years ago
For what it's worth, I've solved this with my own implementation, by hacking
createCSS() a little bit. Basically, the strategy is, for *all* browsers (not
just
IE), to always try to append a style rule from createCSS to an existing
stylesheet/style-block (in the HEAD), if possible, and only create a new one if
one
doesn't already exist.
This gets around the need to "batch" the updates together, as it always ensures
that
this function is not responsible for creating any more than one style
block/stylesheet for all the times it is called. This also (according to my
memory
profiling) seems to reduce the amount of memory usage, by reducing the number
of
extraneous "style" elements appended to the DOM, and of course, solves the
problem
that this function creates for IE6. :)
Here's my hacked version of createCSS():
function createCSS(sel, decl) {
if (ua.ie && ua.mac) {
return;
}
if (!(ua.ie && ua.win) || (ua.ie && ua.win && (doc.styleSheets === UNDEF ||
doc.styleSheets.length <= 0))) {
var h = doc.getElementsByTagName("head")[0], s = null, tmp = null;
if ((tmp=h.getElementsByTagName("style")).length <= 0) { // no style
elements found to append to, just create one
s = createElement("style");
s.setAttribute("type", "text/css");
s.setAttribute("media", "screen");
}
else s = tmp[tmp.length-1]; // just append to the last style
element in the HEAD
if (!(ua.ie && ua.win) && typeof doc.createTextNode != UNDEF) {
s.appendChild(doc.createTextNode(sel + " {" + decl + "}"));
}
if (tmp.length<=0) h.appendChild(s); // style had to be created, so
append it to the DOM now
}
if (ua.ie && ua.win && typeof doc.styleSheets != UNDEF &&
doc.styleSheets.length > 0) {
var ls = doc.styleSheets[doc.styleSheets.length - 1];
if (typeof ls.addRule == OBJECT) {
ls.addRule(sel, decl);
}
}
}
Original comment by get...@gmail.com
on 27 Oct 2008 at 9:18
Hi Kyle, good feedback :-)
The original idea in first versions of SWFFix was to only create one dynamic
style
sheet per web page that defined a class to hide content, and to add and remove
this
class name for the toggle. However, from what I can remember, we ran into some
issues
with older IE versions that these dynamic style rules somehow weren't applied
correctly.
Any way, for a next version I like to revisit this mechanism, and I like your
suggestion. However, what happens in your adjusted version if there is already a
style sheet for a different media type?
Original comment by bobbyvandersluis
on 28 Oct 2008 at 11:20
Good point on the media-type checking. I'll adjust my code hack to account for
that, and I'll post here when I've got updated code.
Original comment by get...@gmail.com
on 30 Oct 2008 at 5:17
OK, here's a revised version of createCSS... it allows you to specify an
optional
parameter for the "media" type... but it defaults to using "screen" if you
don't.
It searches for the last-loaded, enabled stylesheet with a matching media
property
(or an empty/null one, which browsers default to "screen" anyway), and adds the
CSS
rule to that one. It only creates a new stylesheet if a matching one to append
to
cannot be found, so at most it should only create one stylesheet per specified
media
type, and all other rules will be appended to that.
-------------------
function createCSS(sel,decl,media) {
if (ua.ie && ua.mac) {
return;
}
var stys = null, stys_media = null;
if (typeof media !== "string" || media === null) { var media = "screen"; }
media = media.toLowerCase();
if (!(ua.ie && ua.win) || (ua.ie && ua.win && (typeof doc.styleSheets ===
UNDEF || doc.styleSheets.length <= 0))) {
var h = doc.getElementsByTagName("head")[0], s = null;
stys = h.getElementsByTagName("style");
for (var i=(stys.length-1);i>=0;i--) {
if (stys[i].disabled) { continue; }
stys_media = ((typeof stys[i].media === "string") ? stys
[i].media : ((typeof stys[i].media === OBJECT && typeof stys[i].media.mediaText
=== "string") ? media.mediaText : null));
if ((media === "screen" && (stys_media === null ||
stys_media === EMPTY)) || (stys_media.toLowerCase() === media)) { // make sure
stylesheet 'media' matches
s = stys[i];
break;
}
}
if (s === null) { // no style elements found to append to,
just create one
s = createElement("style");
s.setAttribute("type", "text/css");
s.setAttribute("media", media);
h.appendChild(s);
}
if (!(ua.ie && ua.win) && typeof doc.createTextNode !== UNDEF) {
s.appendChild(doc.createTextNode(sel+" {"+decl+"}"));
}
}
if (ua.ie && ua.win && typeof doc.styleSheets !== UNDEF &&
doc.styleSheets.length > 0) {
stys = doc.styleSheets;
for (var j=(stys.length-1);j>=0;j--) {
if (stys[j].disabled) { continue; }
stys_media = ((typeof stys[j].media === "string") ? stys
[j].media : ((typeof stys[j].media === OBJECT && typeof stys[j].media.mediaText
=== "string") ? media.mediaText : null));
if (((media === "screen" && (stys_media === null ||
stys_media === EMPTY)) || (stys_media.toLowerCase() === media)) && (typeof stys
[j].addRule !== UNDEF)) { // make sure stylesheet 'media' matches
stys[j].addRule(sel, decl);
break;
}
}
}
}
Original comment by get...@gmail.com
on 7 Nov 2008 at 5:41
btw, on line 6 of my code, one quick note... change "var media =..." to just
"media
=..." -- more proper JS not to redefine the variable.
Original comment by get...@gmail.com
on 7 Nov 2008 at 5:53
SWFObject 2.2 alpha 3 now only generates 1 dynamic stylesheet and adds the style
rules to this specific stylesheet only.
The code:
function createCSS(sel, decl) {
if (ua.ie && ua.mac) { return; }
if (!dynamicStylesheet) { // create dynamic stylesheet + get a global reference to it
var s = createElement("style");
s.setAttribute("type", "text/css");
s.setAttribute("media", "screen");
dynamicStylesheet = doc.getElementsByTagName("head")[0].appendChild(s);
if (ua.ie && ua.win && typeof doc.styleSheets != UNDEF && doc.styleSheets.length >
0) {
dynamicStylesheet = doc.styleSheets[doc.styleSheets.length - 1];
}
} // add style rule
if (ua.ie && ua.win) {
if (dynamicStylesheet && typeof dynamicStylesheet.addRule == OBJECT) {
dynamicStylesheet.addRule(sel, decl);
}
}
else {
if (dynamicStylesheet && typeof doc.createTextNode != UNDEF) {
dynamicStylesheet.appendChild(doc.createTextNode(sel + " {" + decl + "}"));
}
}
}
Original comment by bobbyvandersluis
on 26 Nov 2008 at 2:39
And dynamicStylesheet is defined as a private variable within the library :-)
Original comment by bobbyvandersluis
on 26 Nov 2008 at 2:40
if dynamicStylesheet is set once at init of the library, are you concerned at
all
that a dynamic page which may add or remove stylesheets during the lifetime of
the
page may have a problem where the stylesheet reference that swfobject tries to
use
is either stale (no longer exists), or has been superceeded (cascading-wise) by
another stylesheet?
I ask because this was the primary motivation for the code of the suggested
function
I put forth, which always finds the "latest" matching stylesheet to add to each
time
it's called. Makes it longer and slightly less performant (barely), but adds
that
extra dimension of robustness for use on really dynamic pages which swap out
stylesheets or other such wizardry.
Original comment by get...@gmail.com
on 26 Nov 2008 at 2:55
Hi Kyle, you're right, it does change the order. I guess I just wanted to save
lines
of code. Will work on a new version :-)
Original comment by bobbyvandersluis
on 26 Nov 2008 at 3:59
What about the following:
/* Cross-browser dynamic CSS creation
*/
function createCSS(sel, decl) {
if (ua.ie && ua.mac) { return; }
var styleSheet = null;
// check if there is an existing stylesheet with media type "screen" that we can reuse
if (typeof doc.styleSheets != UNDEF) {
var s = doc.styleSheets,
sl = s.length;
if (sl > 0) {
for (var i = sl - 1; i >= 0; i--) {
if (!s[i].disabled && ((typeof s[i].media === "string" &&
s[i].media.toLowerCase() === "screen") || (typeof s[i].media === OBJECT &&
typeof
s[i].media.mediaText === "string" && s[i].media.mediaText.toLowerCase() ===
"screen"))) {
styleSheet = s[i];
break;
}
}
}
}
// no stylesheet can be found for reuse: create a dynamic stylesheet
if (!styleSheet) {
var st = createElement("style");
st.setAttribute("type", "text/css");
st.setAttribute("media", "screen");
styleSheet = doc.getElementsByTagName("head")[0].appendChild(st);
if (ua.ie && ua.win && typeof doc.styleSheets != UNDEF && doc.styleSheets.length >
0) {
styleSheet = doc.styleSheets[doc.styleSheets.length - 1];
}
}
// add style rule
if (ua.ie && ua.win) {
if (styleSheet && typeof styleSheet.addRule === OBJECT) {
styleSheet.addRule(sel, decl);
}
}
else {
if (styleSheet && typeof doc.createTextNode != UNDEF) {
styleSheet.appendChild(doc.createTextNode(sel + " {" + decl + "}"));
}
}
}
Original comment by bobbyvandersluis
on 26 Nov 2008 at 5:09
i think this is good in a lot of ways... Firstly, I've noticed from your code
that
my suggested code has some big issues to deal with as well. So it probably
should be
trashed. :)
some quick comments on your code:
1. on media-type testing: in IE (at least), a stylesheet which has no explicit
media
type declared on it defaults to "screen" -- so I accounted for the empty ""
value in
that property when searching for a suitable "screen" style-block/stylesheet to
append rules to. Your code will only append to an explicitly "screen" defined
stylesheet,
My guess is in common practice, your method will still likely end up ignoring a
lot
of default-valued stylesheets and always create a new stylesheet (I think a lot
of
people still don't explicitly declare media types on stylesheet links). If
however
you allow for an empty value for media-type, you can latch on to the default
media-
type behavior and save the overhead of an extra (somewhat unnecessary)
stylesheet.
2. I'm not sure if doc.styleSheets is able cross-browser to pick up on <style>
blocks or not. I thought it *wasn't* able to, which is why I went with
searching for
elements-by-tag-name of "style". If it *is* able to pick up on only a bare
style
block, alls well.
But if not (which I thought was the case), then your method only allows for
adding
rules to proper (externally linked/imported) stylesheets, but ignores just bare
style blocks.
The main issue with this is that style blocks (even though they probably
shouldn't
be used at all) can still be used to override (cascading-wise) rules in an
"earlier"
defined stylesheet, if the style block comes later in the HEAD (or even body of
document).
So, for instance, say a <style> block in the body defines a visibility
attribute for
some DOM object (like the target 'div'). Then no amount of calls to your
createCSS
function will be able to affect that value, because the style block, by virtue
of
being defined "last" will always cascade/override any rules appended to the end
of
an "earlier" stylesheet.
I think for createCSS to truly be a dynamic CSS function, it should always
endeavor
to attach its rules to the "latest" item in the DOM that is style/stylesheet
capable
and of the correct (or default -- see comment #3) media type.
3. I know my suggested function had the additional overhead of allowing an
explicit
media-type for adding a CSS rule to... and in most cases, this is probably
unnecessary complication for swfobject's purposes, which is probably why you
elected
not to include it! :)
but, it may also be helpful for those authors who are using swfobject to hide a
flash from a print-view and instead show alternate content, for example. It
might be
helpful for them to be able to, in combination with the dynamic publishing
methods
probably, dynamically define CSS rules for print versus screen in that
scenario, and
allowing them to pick the media type they want to target would assist greatly
in
that. Just something to think about.
Original comment by get...@gmail.com
on 26 Nov 2008 at 5:45
Great discussion, I love it :-)
Re 1: Correct, no media type means that it defaults to screen, and these should
then
be included as well.
Re 2: Totally correct here, this doesn't work. I tested the code and saw it
didn't
work, removed my comment, and in the meantime you had already commented on it
:-b So
I unremoved it. I was reading through my old dev notes, and correct, DOM
methods are
the way to go for non-IE and the collections for IE.
Regarding the cascade, you can always override declarations by using more
specific
selector rules or the !important keyword.
Re 3: You already guessed it. The main purpose for this function is to
initially hide
alternative content (that's why we focus on the head element only) and help SWF
authors style their 'duplicate' content, however this is an extra. We intend to
keep
things simple.
Furthermore, thinking about media types, this stuff appears to be far more
complex
than we originally thought. E.g. If the last style sheet serves multiple media
types
_including_ screen, and we _only_ like to serve to the screen media type, we
would
have to add a dynamic style sheet after all.
I think that the code in comment 10 - besides that it currently doesn't work -
is a
step into the wrong direction.
So let's take one step back. The original 2.1 concept works correct, however is
a bit
wasteful, because it keeps on adding dynamic style sheets that can ultimately
cause
issues in IE6.
The 2.2 alpha3 approach (comment 6) is short and conceptually correct, however
-
like your comment - we should check if it has been superceeded by another
default or
screen media style sheet. So I think we can best expand the 2.2 alpha3 approach
with
a check if the last element in the head of the document is indeed our dynamic
style
sheet or something else. Regarding the staleness, this is should already be
covered,
because we constantly check for dynamicStylesheet.
Original comment by bobbyvandersluis
on 26 Nov 2008 at 9:29
And now I think of comment 8 again:
'if dynamicStylesheet is set once at init of the library, are you concerned at
all
that a dynamic page which may add or remove stylesheets during the lifetime of
the
page may have a problem where the stylesheet reference that swfobject tries to
use
is either stale (no longer exists), or has been superceeded (cascading-wise) by
another stylesheet?'
No, we shouldn't be concerned about this, however should very precisely
document what
the method is for and when to use it. I think that the 2.2 alpha3 approach is
correct
as it is.
Original comment by bobbyvandersluis
on 26 Nov 2008 at 9:54
The following solution should have it all:
1. An economic way of managing dynamic styles for SWFObject core functionality
2. Re-usable power for SWFObject JavaScript developers - putting the
responsibility
of use in the hands of developers. For this I have added two parameters on top
of the
currect API:
swfobject.createCSS(selectorStr, declarationStr, mediaStr, newStyleBoolean):void
selectorStr - Required
declarationStr - Required
mediaStr - Optional, default value is "string"
newStyleBoolean - Optional, default value is false, forces the creation of a new
style element. SWFObject by default attempts to append it to its last
dynamically
generated style element (conditions: it has created a style element before with
the
exact same media type definition).
The code:
/* Cross-browser dynamic CSS creation
- Based on Bobby van der Sluis' solution:
http://www.bobbyvandersluis.com/articles/dynamicCSS.php
*/
function createCSS(sel, decl, media, newStyle) {
if (ua.ie && ua.mac) { return; }
var m = (media && typeof media == "string") ? media : "screen";
if (newStyle) {
dynamicStylesheet = null;
dynamicStylesheetMedia = null;
}
if (!dynamicStylesheet || dynamicStylesheetMedia != m) {
// create dynamic stylesheet + get a global reference to it
var s = createElement("style");
s.setAttribute("type", "text/css");
s.setAttribute("media", m);
dynamicStylesheet = doc.getElementsByTagName("head")[0].appendChild(s);
if (ua.ie && ua.win && typeof doc.styleSheets != UNDEF && doc.styleSheets.length >
0) {
dynamicStylesheet = doc.styleSheets[doc.styleSheets.length - 1];
}
dynamicStylesheetMedia = m;
}
// add style rule
if (ua.ie && ua.win) {
if (dynamicStylesheet && typeof dynamicStylesheet.addRule == OBJECT) {
dynamicStylesheet.addRule(sel, decl);
}
}
else {
if (dynamicStylesheet && typeof doc.createTextNode != UNDEF) {
dynamicStylesheet.appendChild(doc.createTextNode(sel + " {" + decl + "}"));
}
}
}
Included in swfobject 2.2 alpha4.
Original comment by bobbyvandersluis
on 27 Nov 2008 at 1:18
With dynamicStylesheet and dynamicStylesheetMedia as private variables.
Original comment by bobbyvandersluis
on 27 Nov 2008 at 1:19
I've added a new test page to the 2.2 test suite:
http://www.bobbyvandersluis.com/swfobject/testsuite_2_2/test_api_createcss2.html
Original comment by bobbyvandersluis
on 27 Nov 2008 at 1:23
I really like this... very simple and straightforward implementation... great
job.
A few quick comments/suggestions:
1. 'mediaStr - Optional, default value is "string"' -- actually, default
is "screen", right? ;-)
2. since we're keeping a reference to the stylesheet element that we created,
it
*should* be safe... but what if somehow an external script removes the style
element
that we had saved, and then another call to createCSS tries to access it. I
think
one extra check to make sure that the saved style element reference is still
valid
and in the DOM would improve the robustness. For instance, I think maybe
wrapping a
try/catch around an attempt to access some DOM property of the reference, like:
try { dynamicStylesheet.childNodes.length; } catch (err) { dynamicStylesheet =
null; }
3. Since we're always creating a style-block element, should we consider
appending
that to the body instead of to the head? That way, at least at the time of
original
call, we're sure it's the "latest" style-related element in the DOM, so it
superceeds for cascading purposes? That might improve its robustness not just
for
SWFObject use but also general web author usage.
Original comment by get...@gmail.com
on 28 Nov 2008 at 2:29
1. This explicitly allows us to test for "screen" later on, otherwise we would
have
to test for both "screen" and "". Also, I am not sure if all modern browsers
correctly have implemented this default behavior.
2. Interesting, because we dynamically create the style element and add it to
the DOM
and get a reference to it, the reference is still valid when some developer
would
remove the style sheet using _removeChild_, only now it is available in memory.
So
testing for dynamicStylesheet is still true, and also nothing has changed to
dynamicStylesheet.childNodes.length. So we can still do whatever we would like
to do,
however no result will be seen on screen.
By testing dynamicStylesheet.parentNode we would either get the head element or
null
(after removal).
Some test code:
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN"
"http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html xmlns="http://www.w3.org/1999/xhtml" lang="en" xml:lang="en">
<head>
<title></title>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1" />
<script type="text/javascript">
var h = document.getElementsByTagName("head")[0];
var s = document.createElement("style");
s.setAttribute("type", "text/css");
s.setAttribute("media", "screen");
var dynamicStylesheet = h.appendChild(s);
dynamicStylesheet.appendChild(document.createTextNode("body { background-color:
green; }"));
alert(dynamicStylesheet.parentNode);
h.removeChild(s);
alert(dynamicStylesheet.parentNode);
h.appendChild(s);
</script>
</head>
<body>
</body>
</html>
Will add this for the next build.
3. No, first of all we want to attach the styles as early as possible, that
means in
the head of a page. Also I think that using inline styles is a bad authoring
practice, so if you would really like to override these styles, you should use
more
specific selectors to override this order.
Original comment by bobbyvandersluis
on 28 Nov 2008 at 3:49
The dynamicStylesheet.parentNode test only works for non-IE browsers, IE uses
the
collection of course.
Original comment by bobbyvandersluis
on 2 Dec 2008 at 11:42
So, I see on the current code you don't do the parentNode check at all. So, it
appears you realized this doesn't work for IE, and so just opted to take it out
altogether.
I'm wondering, though, could the check (for non-IE) of parentNode be preserved,
and
for IE, the check is different (like looking to see if our reference is still
in the
collection or something)? This would preserve the robustness of checking if
we're
trying to add to a "stale" stylesheet that has been removed by some other code
and
swfobject is unaware of the change?
I know there are some agressive CMS's out there which manage stylesheets kinda
overboard like that, and so I'm just concerned that maybe our code might not
work so
well with some of them if we're relying on a reference to a stylesheet that no
longer exists for the page.
Original comment by get...@gmail.com
on 9 Dec 2008 at 10:18
RE: So, I see on the current code you don't do the parentNode check at all.
So, it
appears you realized this doesn't work for IE, and so just opted to take it out
altogether.
Indeed, and also I am not convinced anymore if this is really a potential issue.
Worst scenario a web author would dynamically remove our dynamic style rule
from the
DOM, in which case nothing crashes, only styles defined by that same author
using
swfobject.creatCSS won't be applied, so this would really be an authoring
issue. Also
I have the opinion that if you have the knowledge to remove srylesheets you
should
pretty much know what you are doing, which includes leaving SWFObject's dynamic
style
rules alone.
RE: I know there are some agressive CMS's out there which manage stylesheets
kinda
overboard like that...
Well, how can a CMS remove a dynamically generated style element (at run time)?
If
you have any concrete examples or code, please let me know.
Original comment by bobbyvandersluis
on 10 Dec 2008 at 9:30
Included in the SWFObject 2.2 beta1 release
Original comment by bobbyvandersluis
on 16 Apr 2009 at 3:05
Hallo, gentleman.
I've encountered an issue with createCSS in IE 8.
(more on attached image)
I haven't found nothing better than wrap error prone code snippet into
try-catch:
try{
if (ua.ie && ua.win) {
if (dynamicStylesheet && typeof dynamicStylesheet.addRule ==
OBJECT) {
dynamicStylesheet.addRule(sel, decl);
}
}
else {
if (dynamicStylesheet && typeof doc.createTextNode != UNDEF)
{
dynamicStylesheet.appendChild(doc.createTextNode(sel
+ " {" + decl + "}"));
}
}
}catch(e){
//alert(e);
}
It's a sort of brutal hack, but it works...
Regards.
Original comment by box4...@gmail.com
on 8 May 2009 at 2:16
Attachments:
Why are you adding this issue to multiple issue reports? Could you please read
the
reply on your original submission.
Original comment by bobbyvandersluis
on 8 May 2009 at 2:23
Oh, really. I didn't noticed. Accept my apologies.
I'll continue in topic for issue 255.
Original comment by box4...@gmail.com
on 9 May 2009 at 9:39
Original issue reported on code.google.com by
get...@gmail.com
on 27 Oct 2008 at 8:38