setOpacity() should have "force" param about prototype HOT 28 CLOSED

Nick Stakenburg
August 20th, 2008 @ 11:01 AM

Setting no opacity fixes cleartype rendering on fonts.
If you know that an element is going to dynamically change opacity, don't set an opacity in css but use setOpacity on the element, or even inline style if you want to get nasty.

from prototype.

@jdalton
August 20th, 2008 @ 04:06 PM

State changed from “new” to “invalid”
I agree with Nick, this is the same issue was elements dont show/hide because devs set their display: in the css.

from prototype.

Christopher Thomas
August 27th, 2008 @ 03:07 PM

the problem with this is that my explict actions are being overriden, I want opacity: 1, not to remove the opacity attribute, that is not the same thing, just because they happen to do the same task, does not make them equal.
if I ask for opacity: 1, I expect that, I dont expect the attribute to be removed. so then a further test later in the code doesnt find the opacity attribute, fantastic, now because my actions are being overriden, I have to code around that fact, or just not use setOpacity.
add an extra parameter to the method removeOpacity, when set to false, it will leave the attribute in place, with value 1.
then everyone is happy.
putting opacity: 0 in stylesheets is a perfectly valid thing to do, since by default, the opacity is 0, fading an element in once per page, I should change my css? maybe is happens once? or perhaps I should just comment out this part of prototype and fork my changes?
you're overriding my explicit request, thats why the OP requested the change he did.
the issue with display: is also the same, I explicitly requested THAT and I want THAT, prototype is in the business of providing a framework for building javascript, NOT for building a policy of how people develop other parts of their website.
You're a javascript frame, nothing more, stop overriding explicit decisions I or someone else made. You're forcing unwanted policy decisions upon me.
THATS where the problem here is.

from prototype.

@tobie
August 27th, 2008 @ 03:28 PM

Title changed from “Element#setStyle should not use setOpacity()” to “setOpacity(1) Removes Opacity”
State changed from “bug” to “invalid”
Tag changed from “bug, opacity, setopacity, setstyle” to “dom”
Milestone cleared.
Thomas, no one is forcing you to use Element#setOpacity. As Nick pointed out above, removing the opacity attribute must be done for proper font rendering. It will thus stay as it is.

from prototype.

@jdalton
August 27th, 2008 @ 03:32 PM

Title changed from “setOpacity(1) Removes Opacity” to “Element#setStyle should not use setOpacity()”
State changed from “invalid” to “bug”
Tag changed from “dom” to “bug, opacity, setopacity, setstyle”
Milestone set to “1.7”
I was wrong about setOpacity.
Because Element#setStyle calls Element#setOpacity you can never set the opacity to 1. This is different from the Element#hide and Element#show because they don't highhack the ability to set the "display" style in setStyle.
Element#setStyle should allow the user to set whatever style they wish.

from prototype.

@tobie
August 27th, 2008 @ 03:34 PM

Tag changed from “bug, opacity, setopacity, setstyle” to “bug, dom, element”
I get your point JDD and it makes sense, however, making setStyle and setOpacity behave differently seems to strongly violate POLS.

from prototype.

Mark Caudill
August 27th, 2008 @ 03:41 PM

Title changed from “Element#setStyle should not use setOpacity()” to “setOpacity(1) Removes Opacity”
Tag changed from “bug, dom, element” to “opacity, setopacity, setstyle”
@chris: Not that I disagree, but the reason the opacity is set to '' is to achieve the greatest possible results, not throw off your programming.
While I understand your plight, prototype is a framework to make decisions that cause the least "bad" from the end user's viewpoint. That is, making it so cleartype works best by clearing the opacity (or filter alpha opacity) instead of setting it to 1 (which, like you said, achieve the same result).
The inherent problem you're experiencing is a trade-off with bugs in browsers, not an actual bug. As suggested, if you need to set it to 1 and it needs to be 1, use .style.opacity (but you lose filter alpha opacity support).

from prototype.

Christopher Thomas
August 27th, 2008 @ 03:44 PM

Title changed from “setOpacity(1) Removes Opacity” to “Element#setStyle should not use setOpacity()”
Tag changed from “opacity, setopacity, setstyle” to “bug, dom, element”
The main central plank here is that I am DEMANDING from the browser that it sets opacity: 1, but as JDD pointed out, you can't do it, unless you return to using NON prototype methods.
Just because of font rendering??
When I fade from 0->1 my font rendering will break with IE, thats obvious, I've seen it before, when the font rendering looks like it has a black pixelated background and really crappy.
But thats MY problem, not YOUR problem.
You're effectively telling me that YOU WILL SOLVE IT, without asking me whether the solution you gave, causes other problems.
policy decisions on my site are made by me, technical implementation decicions of prototype, are made by you, you cannot cross over, say, I'm going to remove that attribute since opacity: 1 and opacity: "" are the same, thats policy, I will tell you whether they are the same or not and in this situation, they are not the same.
so, nobody is forcing me to use it, thats true, there is a "workaround" thats true, but I would prefer that you dont tread on my toes.
opacity: 1 is NOT the same as opacity: ""
font rendering issues are a separate problem, with a separate solution (such as, dont use opacity, or AFTER I set opacity: 1, I can explicity request opacity: "", but thats MY choice.
you see my point?

from prototype.

Nick Stakenburg
August 27th, 2008 @ 03:46 PM

Tag changed from “bug, dom, element” to “dom”
I don't think we need to change anything. Like with display we force people to code properly, why not do the same with opacity?
I think that's the right thing todo. If people decide to use a framework they'll have break away from bad habbits. Or do we need to provide people with ways to break things? If we change this the next issue that will come up is why does setOpacity(1) break my fonts.

from prototype.

Mark Caudill
August 27th, 2008 @ 03:49 PM

I'm going to agree with others on this one, Chris. The fact is, if setOpacity is used with the intent of not breaking cleartype fonts (often), and your case is to break cleartype fonts but maintain the opacity (stated), then you could have the best of both worlds by checking to see if opacity is set before you assume it's 1.

from prototype.

Christopher Thomas
August 27th, 2008 @ 03:50 PM

Tag changed from “dom” to “bug, dom, element”
@mark
true, whilst the idea is to sidestep, workaround and solve browser bugs, the only problem here is that font rendering appears to be broken, which will be the case whilst opacity > 0 && < 1 so therefore, it will be broken during the translation and then fixed at the end.
but, this is a separate problem from my pov, font rendering issues are solved by setting opacity: "" and if I have them, I will do that, but in this case, I dont have that problem and most of the situations I've faded, I dont have the problem either, so I'm seeing a solution which for me at least, solves ontly a fraction of the situations I am faced with. Hence it appears to fix a minority problem whilst creating a larger one, that prototype is now making me do extra work to get around "bugs" in prototype, I do see this as a bug, because you are affecting a style to fix a bug in something else, but the fix, causes problems. hence you fix one "bug" by creating another.
so, if I have font rendering issues, I'll do my loop, finalise with say setOpacity(1) and then after the loop, I'll set opacity to "" myself
the solution is probably to add an optional parameter which allows me to disable or enable this behaviour, then at least I can do BOTH things.

from prototype.

Christopher Thomas
August 27th, 2008 @ 03:57 PM

@nick
there is no bad habit involved here, I am not sure where you got that idea from, setting opacity: 0 into a stylesheet for something that will appear once, twice in a page seems perfectly normal. The world isnt just about javascript. What do I do for people without javascript serve up a different stylesheet? which fixes the fact that prototype forced me NOT to put opacity: 0 into it therefore now you create the problem in a different place.
The problem is not bad habits, it's assuming control over things which are none of prototypes business, get off my lawn so to speak, not in a rude sense, but you shouldnt be touching things which are not your business without a good reason. Setting opacity: "" to fix font rendering sounds like a great idea, until you consider the problems it might create.
Bad habits? thanks to IE, the web is full of them and there are hacks in prototype just like every other website has to do in order to get the desired results, ever used a star-hack? uh oh! bad habits!! but we're forced to do it. Hell, not that I have a specific example, but I am sure that prototype does some crazy things in order to get a level playing field and someone, somewhere will say "Thats a bad habit!"
So I dont think throwing bad habits in here is useful

from prototype.

Christopher Thomas
August 27th, 2008 @ 04:00 PM

@mark,
checking to see if the opacity is 1 before I set it doesnt help in this case though from what I can see, if the opacity is set to 0 in the stylesheet, how can I use setOpacity(1) ? I can't, because if in the stylesheet the opacity is 0, then setOpacity(1) actually does opacity: 0
the final result is the same.
if in the stylesheet, the opacity is 0 setOpacity(1) will actually set opacity: "" which is in this case, exactly the same as opacity: 0,
hence, setOpacity(1) is NOT doing what I expect. It is doing the direct opposite.

from prototype.

@jdalton
August 27th, 2008 @ 04:10 PM

Tag changed from “bug, dom, element” to “dom”
Ahh I forgot that Element#setStyle uses setOpacity to give IE, Safari, and versions of Gecko support. Hmmmm....
OT: I did find that the IE fork of setOpacity could be cleaned up a bit
@christopher Thomas - remove opacity from your stylesheet. Its not cross browser anyway (IE6 at least).

from prototype.

Nick Stakenburg
August 27th, 2008 @ 04:11 PM

Tag changed from “dom” to “bug, dom, element”
Christopher: Solutions for your problem have been provided, try to look at this issue from a broader perspective instead of only trying to fix your current issue wich is easily solved.
Again, if we fix this the next issue will be why does setOpacity(1) break my fonts. That problem is not easilly solved if we decide to change this.
There's nothing wrong with setting opacity in your css, just as there's nothing wrong with setting display in your css. But don't expect a framework that's attempting to give everyone the best possible experience to work with your setup out of the box.
When you use a framework you have to be flexible with code you write around it, we are simply trying to provide you with the best cross browser solution. The solutions provided will give you the best possible experience with Prototype.

from prototype.

Mark Caudill
August 27th, 2008 @ 04:11 PM

@chris: We do not have access to the stylesheet information. This is much the reason why you should not bury the opacity information in CSS if you are going to be modifying it at any point. It is not .setOpacity's fault that you're having problems, it's your insistence on hiding the information from JavaScript.

from prototype.

Christopher Thomas
August 27th, 2008 @ 04:13 PM

I think you're solving the wrong problem and you're doing it wrong at the same time.
I'll modify my prototype library to fix it.
thanks for the insight.

from prototype.

Christopher Thomas
August 27th, 2008 @ 05:05 PM

no changes were found...

Index: /Volumes/CLIENTES/HARLEY/BBQ_SERIES/WEB_BBQ/2_WORKING/development/binamic/javascript/prototype.js
===================================================================
--- /Volumes/CLIENTES/HARLEY/BBQ_SERIES/WEB_BBQ/2_WORKING/development/binamic/javascript/prototype.js   (revision 166)
+++ /Volumes/CLIENTES/HARLEY/BBQ_SERIES/WEB_BBQ/2_WORKING/development/binamic/javascript/prototype.js   (working copy)
@@ -1925,9 +1925,9 @@
     return element;
   },

-  setOpacity: function(element, value) {
+  setOpacity: function(element, value, force) {
     element = $(element);
-    element.style.opacity = (value == 1 || value === '') ? '' :
+    element.style.opacity = (!force && (value == 1 || value === '')) ? '' :
       (value < 0.00001) ? 0 : value;
     return element;
   },
@@ -2266,7 +2266,7 @@
     return value;
   };

-  Element.Methods.setOpacity = function(element, value) {
+  Element.Methods.setOpacity = function(element, value, force) {
     function stripAlpha(filter){
       return filter.replace(/alpha\([^\)]*\)/gi,'');
     }
@@ -2278,8 +2278,10 @@

     var filter = element.getStyle('filter'), style = element.style;
     if (value == 1 || value === '') {
-      (filter = stripAlpha(filter)) ?
-        style.filter = filter : style.removeAttribute('filter');
+      if(!force){
+       (filter = stripAlpha(filter)) ?
+         style.filter = filter : style.removeAttribute('filter');
+      }
       return element;
     } else if (value < 0.00001) value = 0;
     style.filter = stripAlpha(filter) +
@@ -2375,7 +2377,7 @@
 }

 else if (Prototype.Browser.Gecko && /rv:1\.8\.0/.test(navigator.userAgent)) {
-  Element.Methods.setOpacity = function(element, value) {
+  Element.Methods.setOpacity = function(element, value, force) {
     element = $(element);
     element.style.opacity = (value == 1) ? 0.999999 :
       (value === '') ? '' : (value < 0.00001) ? 0 : value;
@@ -2384,9 +2386,9 @@
 }

 else if (Prototype.Browser.WebKit) {
-  Element.Methods.setOpacity = function(element, value) {
+  Element.Methods.setOpacity = function(element, value, force) {
     element = $(element);
-    element.style.opacity = (value == 1 || value === '') ? '' :
+    element.style.opacity = (!force && (value == 1 || value === '')) ? '' :
       (value < 0.00001) ? 0 : value;

     if (value == 1)

from prototype.

Christopher Thomas
August 27th, 2008 @ 05:05 PM

Title changed from “Element#setStyle should not use setOpacity()” to “setOpacity() should not remove the attribute when opacity is set to 1”
I created a patch for the problem I described above, it's a very simple change, if anyone can find a problem with it, let me know.
basically, it changes setOpacity from taking one argument, to optionally taking two, the second, called "force" will not override the expected behaviour to fix font issues.
so therefore, is opacity: 1 is REALLY what you want, you can set force=true or any non false value (like if(force), any success code here will enabled force, so, 1, 1.1, "your mother" or "true")
Examples:
element.setOpacity(1);
will do the default, set opacity: ""
element.setOpacity(1,true);
will set opacity: 1
element.setOpacity(1,false);
will do the default, set opacity: ""
this has been given the limited testing I can give it (run in firefox 2, 3, safari 2, internet explorer 6 and 7)
obviously more testing is better, if anyone wants to help out.
thanks!

from prototype.

@jdalton
August 27th, 2008 @ 05:59 PM

Title changed from “setOpacity() should not remove the attribute when opacity is set to 1” to “setOpacity() should have "force" param”
State changed from “new” to “enhancement”
Milestone cleared.
The Prototype.Browser.Gecko && /rv:1.8.0/.test(navigator.userAgent) fork doesn't really bogart the value so the force param isn't even used for that fork.

from prototype.

Christopher Thomas
August 27th, 2008 @ 06:06 PM

so I should remove the force parameter from that method? is that what you're suggesting?

from prototype.

@jdalton
August 27th, 2008 @ 06:10 PM

ya, I mean it could be there to show intent or consistent api, but really its not needed.

from prototype.

Christopher Thomas
August 27th, 2008 @ 06:13 PM

ok, well, since you mentioned it would show a consistent api, I'll leave it there

from prototype.

Christopher Thomas
February 9th, 2010 @ 10:36 PM

is there any progress on getting this bugfix into prototype anytime soon?
it fixes the relevent problem allowing the user to override what prototype wants to do with what the website owner wants to do, allowing both behaviours to sit side by side (without side effects).
can you let me know what the status is please, thanks! I dont want to keep having to apply this patch to my code each time I update prototype

from prototype.

@jwestbrook are you a bot?

from prototype.

@jdalton no I'm migrating tickets from PrototypeJS Lighthouse to github issues

from prototype.

Cool. Can you [@]jdalton for my references so I don't get pinged every time. Thanks.

from prototype.

@jdalton no problem

from prototype.