Bug Tracker

Ticket #2954 (closed bug: fixed)

Opened 7 years ago

Last modified 3 years ago

Use isNaN for IE opacity

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

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

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

Change History

Changed 7 years ago by lrbabe

patch to replace the test with isNaN()

comment:1 Changed 7 years ago by flesler

  • Owner set to flesler
  • Status changed from new to assigned

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

comment:2 Changed 7 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 7 years ago by lrbabe

oops oops oops, never mind my last comment.

comment:4 Changed 7 years ago by lrbabe

... actually you can mind it.

comment:5 Changed 7 years ago by flesler

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

comment:6 Changed 6 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 4 years ago by dmethvin

  • Component changed from core to css
  • Summary changed from [patch] Use isNaN if appropriate to Use 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 4 years ago by dmethvin

  • Status changed from assigned to closed
  • Resolution set to fixed

Implemented in the rewrite of csshooks in 1.4.3.

Note: See TracTickets for help on using tickets.