-
Notifications
You must be signed in to change notification settings - Fork 640
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
setOpacity() should have "force" param #145
Comments
Nick Stakenburg Setting no opacity fixes cleartype rendering on fonts. |
@jdalton State changed from “new” to “invalid” |
Christopher Thomas 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. |
@tobie Title changed from “Element#setStyle should not use setOpacity()” to “setOpacity(1) Removes Opacity” |
@jdalton Title changed from “setOpacity(1) Removes Opacity” to “Element#setStyle should not use setOpacity()” |
@tobie Tag changed from “bug, opacity, setopacity, setstyle” to “bug, dom, element” |
Mark Caudill Title changed from “Element#setStyle should not use setOpacity()” to “setOpacity(1) Removes Opacity” |
Christopher Thomas Title changed from “setOpacity(1) Removes Opacity” to “Element#setStyle should not use setOpacity()” |
Nick Stakenburg Tag changed from “bug, dom, element” to “dom” |
Mark Caudill 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. |
Christopher Thomas Tag changed from “dom” to “bug, dom, element” |
Christopher Thomas @nick |
Christopher Thomas @mark, |
@jdalton Tag changed from “bug, dom, element” to “dom” |
Nick Stakenburg Tag changed from “dom” to “bug, dom, element” |
Mark Caudill @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. |
Christopher Thomas I think you're solving the wrong problem and you're doing it wrong at the same time. |
Christopher Thomas 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) |
Christopher Thomas Title changed from “Element#setStyle should not use setOpacity()” to “setOpacity() should not remove the attribute when opacity is set to 1” |
@jdalton Title changed from “setOpacity() should not remove the attribute when opacity is set to 1” to “setOpacity() should have "force" param” |
Christopher Thomas so I should remove the force parameter from that method? is that what you're suggesting? |
@jdalton ya, I mean it could be there to show intent or consistent api, but really its not needed. |
Christopher Thomas ok, well, since you mentioned it would show a consistent api, I'll leave it there |
Christopher Thomas is there any progress on getting this bugfix into prototype anytime soon? |
@jwestbrook are you a bot? |
@jdalton no I'm migrating tickets from PrototypeJS Lighthouse to github issues |
Cool. Can you [@]jdalton for my references so I don't get pinged every time. Thanks. |
@jdalton no problem |
previous lighthouse ticket #293
by dave mankoff
For elements that have an opacity of non-1 set in their stylesheet, calls to setOpacity(1) do not behave as expected - they simply set the opacity to that of the stylesheet.
I think a simple fix would be to remove the value == 1 check.
The text was updated successfully, but these errors were encountered: