Fix returns type of css value.

[ghost]

Such property as zIndex for IE returns from window.getComputedStyle value of number type. I think that the value should have the unified type, when it returns.

[dmethvin]

If you're going to assert that .css() should always return a string, you also need a unit test to ensure it doesn't regress.

[mgol]

Even IE11 has this bug. :/

[mgol]

I reported it to Microsoft: https://connect.microsoft.com/IE/feedback/details/804689/getcomputedstyle-returns-an-object-where-not-all-values-are-strings

[ghost]

@dmethvin thanks. You are right!

[dmethvin]

The new commit doesn't follow the style guide. Are you running grunt locally before pushing your changes to the pull request? You should do that and test locally as well.

We still need one or more unit tests to specify what this commit is trying to assert as far as API output is concerned.

[ghost]

@dmethvin I've fixed the code according to the style guide and added unit test.

[mgol]

@katsgeorgeek We also need a unit test confirming that your change works, i.e. that .css('zIndex') does return "1", not 1.

[ghost]

@mzgol Does the code correspond to the code-style now?

[gibson042]

I'm not sure this change is worthwhile... IE is wrong here, but what application of the difference between a number and a numeric string justifies the performance hit of additional operations in a function at the heart of quite a few tight loops (e.g., for animation)?

[ghost]

@gibson042 With equal quantity and type of data-in we should always get return value of a one type. If the performance was the most important I would use VanillaJS :) In my opinion jQuery helps to hide differences in implementation of browsers.

[gibson042]

@katsgeorgeek I accept your "ought" proposition, but see closed: patchwelcome for documented examples where we fail to do what we should. Do you have an example of realistic code that breaks when .css("z-index") returns a Number?

[ghost]

@gibson042 For example, photo collage, I need to check z-index to move between the layers.

[scottgonzalez]

@katsgeorgeek How does you code break in that situation? Aren't you just doing a parseInt() on the value?

[ghost]

@scottgonzalez Why should I do something?

[scottgonzalez]

@katsgeorgeek I don't understand your question. You need numbers in order to actually do anything. But you're complaining that you're getting numbers and not a string. If .css() did the right thing and returned a string, you'd have to parseInt(). When .css() does the wrong thing and returns a number, using parseInt() on the value is perfectly fine. So what's the problem? You haven't explained how you're actually running into problems with the current behavior.

[ghost]

@scottgonzalez

1. It doesn't matter what type is the value. B/c I can compare strings
2. I described the issue at PR. IE returns incorrect type of value, but Chrome, FF etc. - correct.

[scottgonzalez]

Your code is broken if you're comparing strings for zIndex. You've described a technical problem, but not a impacts-real-code problem.

[ghost]

@scottgonzalez I haven't said that I'm checking, I've said "I can check". Indeed, I found this bug while was refactoring legacy code. All the same the check $('.test').css('s-index') === '-1' is possible and programers don't need to keep in their minds that IE will return the value with another type.

[scottgonzalez]

Thank you for finally providing real code that is affected by this.

[banzalik]

Sorry, but, http://api.jquery.com/css/ say .css( propertyName ) Returns: String

[scottgonzalez]

Nobody is debating that. We all agree that the patch is correct. @gibson042 asked if the performance hit is worth it considering this is the first time anyone has reported this problem (presumably this problem has existed for almost 8 years). And it was like pulling teeth to get an answer to his simple question "Do you have an example of realistic code that breaks when .css("z-index") returns a Number?"

[gibson042]

I suspect the hit will in fact be small, but it should still be quantified given the low impact of this bug. @katsgeorgeek, can you put together a jsPerf showing the difference between returning ret and ret === undefined ? ret : ret + "" when ret is a String or Number?

[ghost]

Sure it's more slowly http://bit.ly/18MIVxJ

[gibson042]

Sure it's more slowly http://bit.ly/18MIVxJ

But as expected, so fast that it really doesn't matter. I'll support this string conversion, at a total cost after optimizations of +8 to jquery.min.js.

[scottgonzalez]

I had @arschmitz run the perf test on a few devices. While some show a massive slow down for ret (Number), I think we really only care about the slow down for ret (String) since that's what will impact the tight loops (I doubt anyone's doing any animation of zIndex).

[gibson042]

Allright then, we can now get the horse in front of the cart. :wink:

@katsgeorgeek, please open a ticket at http://bugs.jquery.com/ and reference it in this PR. Then it can land as soon as the code meets our requirements.

[ghost]

@gibson042 I've created bug report and changed code according to requirements.

http://bugs.jquery.com/ticket/14432

[ghost]

@gibson042 Is something else that prevent accepting this PR?

[gibson042]

This is looking good; I'll probably land it sometime tomorrow unless someone beats me to it.

[gibson042]

I was about to land this when I saw that you haven't signed our CLA. Please do so at your earliest convenience.

[ghost]

@gibson042 done