ability to retrieve uncomputed styles in IE<9, fixes #10796

[mikesherov]

Some people, when confronted with a problem, think "I know, I'll use regular expressions." Now they have two problems.

But seriously, I suck at them. Please let me know if there is a less complex regex that fixes this bug.

[gibson042]

Unfortunately, rnumnopx fails to recognize real-number values like "4.2%" and explicit positive values like "+1ex" (the latter flaw shared with rnumpx), and gives a false positive on "1em center". Sticking with the "!numpx && numnopx" approach, you could define rnumnopx = /^[-+]?\d*(?:\d|\.\d+)(?!px)[^\d\s]+$/, but I think it might be better to define rnum = /^[-+]?\d*(?:\d|\.\d+)([^\d\s]+)?$/ (capturing the units at index 1) and then use ( ( rnum.exec( ret ) || [] )[1] || "px" ) !== "px" (passing only if rnum matches and units are non-px) in place of the test on line 300.

Sorry if that doesn't help with your "less complex" request.

http://www.w3.org/TR/CSS21/syndata.html#numbers
http://www.w3.org/TR/CSS21/colors.html#propdef-background-position

[mikesherov]

This is great! Thanks for the help.

[mikesherov]

Thinking about this further, we can take hybrid approach: one regex
without having to capture that looks for number followed by a string
that isn't px and contains no spaces followed by end of string.

Sent Via Mobile: Please excuse my grammar, tone, and punctuation. My
thumbs can't create flowery prose.

On Nov 20, 2011, at 5:38 AM, Richard Gibson
reply@reply.github.com
wrote:

@@ -5,7 +5,7 @@ var ralpha = /alpha([^)]*)/i,
// fixed for IE9, see #8346
rupper = /([A-Z]|^ms)/g,
rnumpx = /^-?\d+(?:px)?$/i,

• rnum = /^-?\d/,
• rnumnopx = /^-?\d+(?!px)[^\d]+$/i,

Unfortunately, rnumnopx fails to recognize real-number values like "4.2%" and explicit positive values like "+1ex" (the latter flaw shared with rnumpx), and gives a false positive on "1em center". Sticking with the "!numpx && numnopx" approach, you could define rnumnopx = /^[-+]?\d*(?:\d|\.\d+)(?!px)[^\d\s]+$/, but I think it might be better to define rnum = /^[-+]?\d*(?:\d|\.\d+)([^\d\s]+)?$/ (capturing the units at index 1) and then use ( ( rnum.exec( ret ) || [] )[1] || "px" ) !== "px" (passing only if rnum matches and units are non-px) in place of the test on line 300.

Sorry if that doesn't help with your "less complex" request.

http://www.w3.org/TR/CSS21/syndata.html#numbers
http://www.w3.org/TR/CSS21/colors.html#propdef-background-position

---

Reply to this email directly or view it on GitHub:
https://github.com/jquery/jquery/pull/610/files#r241007

[gibson042]

My pleasure. I like the hybrid idea of independently using "number (pixels)" and "number (non pixels)" regular expressions.

[mikesherov]

@gibson042, I went ahead and used our combined approach by making it into 1 regex. Thanks!

Regarding "+12em", IE doesn't report the "+" sign there for that hack, so I don't need to check for it. Also, regarding "12.2em", the goal of this PR wasn't to fix the cases were "12.2em" WASN'T getting converted to equivalent pixels, so I'll save THAT exercise for another day.

[mikesherov]

for those curious:

jQuery Size - compared to last make
  248155     (+1) jquery.js
   93853     (+3) jquery.min.js
   33142     (+6) jquery.min.js.gz

if we got rid of Dean Edwards awesome hack:

jQuery Size - compared to last make
  247379   (-776) jquery.js
   93631   (-222) jquery.min.js
   33053    (-89) jquery.min.js.gz

[gibson042]

I'm totally new to this process, but understand the benefit of focused commits. Should I open a ticket for the currentStyle function's inability to recognize non-integer numeric values?

[mikesherov]

Sure, you should head on over to bugs.jquery.com and follow the instructions for submitting a bug there. After that, you can submit a pull request that fixes the bug. See here for more info: http://docs.jquery.com/Getting_Involved and also read the readme here: https://github.com/jquery/jquery

[mikesherov]

@gibson042, you can also head over the the #jquery-dev irc channel for more guidance: http://webchat.freenode.net/?channels=jquery-dev

[gibson042]

Hmm... the old test was only on "starts like a number", so the issue with real-number values won't manifest until after this PR is integrated. I think it should be addressed now.
rnumnopx = /^-?\d*(\d|\.\d+)(?!px)[^\d\s]+$/i

[mikesherov]

@gibson042, please submit a bug outlining the issue. Then we can work on some unit tests in another PR based off of this.

[gibson042]

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

[gibson042]

http://jsperf.com/2011-11-23-regexp-complexity
There doesn't seem to be any measurable performance change.

[mikesherov]

Fair enough, I'll roll your change in then.

[gibson042]

Upon reading the jsperf documentation, it turns out that I misunderstood setup. I've corrected the issue and checked again... there is a performance hit to testing strings against the longer expression, but it's generally small (most significant on Chrome at about 10-15%).

http://jsperf.com/2011-11-23-regexp-complexity

Sorry about so much attention on a minor issue, and big thanks for taking me under your wing.

[mikesherov]

Probably better to build a copy of jQuery that has your new regex in
there, and test actual .css function calls in IE<9 that would hit the
regex. Also, testing for explicit + values is not necessary as IE<9
won't have explicit + signs. Lastly, I believe this isn't the most
optimized regex

Sent Via Mobile: Please excuse my grammar, tone, and punctuation. My
thumbs can't create flowery prose.

On Nov 24, 2011, at 1:31 PM, Richard Gibson
reply@reply.github.com
wrote:

Upon reading the jsperf documentation, it turns out that I misunderstood setup. I've corrected the issue and checked again... there is a performance hit to testing strings against the longer expression, but it's generally small (most significant on Chrome at about 10-15%).

http://jsperf.com/2011-11-23-regexp-complexity

Sorry about so much attention on a minor issue, and big thanks for taking me under your wing.

---

Reply to this email directly or view it on GitHub:
#610 (comment)

[gibson042]

http://jsperf.com/2011-11-23-regexp-complexity/2

Assuming I've set up the tests correctly, there is approximately a 4% difference in .css(property) speed between all candidate expressions, including the integer-only variant (which was fastest by 1%). This is comparable to the per-test variability reported by jsperf.

I recommend using the shortest of the three decimal-accepting expressions, which also tested fastest:
rnumnopx = /^-?\d*(?:\d|\.\d+)(?!px)[^\d\s]+$/i
rnumpx = /^[-+]?\d*(?:\d|\.\d+)(?:px)?$/i

[mikesherov]

@gibson042, let's move this conversation over to the trac ticket for the issue you raised. At this point, it's way off topic.

[dmethvin]

Landed. 6aa4095