Bug Tracker

Opened 15 years ago

Closed 13 years ago

Last modified 11 years ago

#2954 closed bug (fixed)

Use isNaN for IE opacity

Reported by: lrbabe Owned by: flesler
Priority: major Milestone: 1.3
Component: css Version: 1.2.5
Keywords: Cc:
Blocked by: Blocking:

Description

function attr of core.js (the second one) use a strange test to detect NaN :

parseInt( value ) + "" == "NaN" ?

If appropriate, replace it with :

isNaN(parseInt(value))?

See proposed patch.

Attachments (1)

patch_nan.txt (639 bytes) - added by lrbabe 15 years ago.
patch to replace the test with isNaN()

Download all attachments as: .zip

Change History (9)

Changed 15 years ago by lrbabe

Attachment: patch_nan.txt added

patch to replace the test with isNaN()

comment:1 Changed 15 years ago by flesler

Owner: set to flesler
Status: newassigned

I wanted to do the same change some weeks ago. But for some reason the results were different.

comment:2 Changed 15 years ago by lrbabe

By the way, if we are just testing if the value is a number, shouldn't we use value.constructor == Number ?

comment:3 Changed 15 years ago by lrbabe

oops oops oops, never mind my last comment.

comment:4 Changed 15 years ago by lrbabe

... actually you can mind it.

comment:5 Changed 15 years ago by flesler

No, because it's a string. So value.constructor == String.

comment:6 Changed 15 years ago by flesler

Right. The change could probably fit. I don't see much gain but might make it in. Will check this asap, it requires a lot of testing.

comment:7 Changed 13 years ago by dmethvin

Component: corecss
Summary: [patch] Use isNaN if appropriateUse isNaN for IE opacity

Here's the current line in 1.4.2:

var opacity = parseInt( value, 10 ) + "" === "NaN" ? "" : "alpha(opacity=" + value * 100 + ")";

parseInt() only looks for valid leading digits that can be followed by non-digits, but isNaN() requires the entire string to be a valid number. Opacity should be a number from 0.0 to 1.0 (not an integer). Also the Microsoft docs say the opacity should be an integer from 0 to 100.

http://msdn.microsoft.com/en-us/library/ms532910%28v=VS.85%29.aspx

So it seems like the correct line would be this:

var opacity = isNaN(value) ? "" : "alpha(opacity=" + Math.round(value * 100) + ")";

comment:8 Changed 13 years ago by dmethvin

Resolution: fixed
Status: assignedclosed

Implemented in the rewrite of csshooks in 1.4.3.

Note: See TracTickets for help on using tickets.