#1331 closed bug (fixed)
Latest Nightlies Crash Safari 1.3.2
| Reported by: | jhenry | Owned by: | |
|---|---|---|---|
| Priority: | critical | Milestone: | 1.1.4 |
| Component: | core | Version: | 1.1.3 |
| Keywords: | Cc: | ||
| Blocked by: | Blocking: |
Description
Version 1.1.2 loads correctly in Safari 1.3.2, but version 1.1.3a and the current nightly builds are crashing Safari 1.3.2 (browser exits) on load with a page as simple as
<html><head><script type="text/javascript" src="jquery.js"></script></head></html>
Attachments (2)
Change History (34)
comment:1 Changed 16 years ago by
comment:2 Changed 16 years ago by
| Version: | → 1.1.3 |
|---|
I don't have a 10.3 machine - nor do I know anyone who has one. Anything that you can do to break it down will help us. The ideal situation would be one where you can say "I can comment out this line and the problem is now fixed."
comment:3 Changed 16 years ago by
I'm not familiar with the jquery codebase, so I'm not sure where I'd start commenting out lines. Could you give me a few svn revision numbers to check out and build during the 1.1.2 to 1.1.3a timeline so I can determine which check-in broke the tree for Safari 1.3.2?
comment:4 Changed 16 years ago by
Well, 1.1.2 was 1465 and 1.1.3a was 1938 - I guess maybe taking a look at every 100 rev or so would be a start, we could drill down from there. You could also do a binary search - split the revisions in half (start at 1700 or so) and if it's broken, go down revision, if it's not broken, go up.
comment:5 Changed 16 years ago by
I started doing some tests on this, and I found that using the uncompressed version of jquery does not crash safari 1.3.2. It's only the packed version that is causing the error. I will find the revision at which the packer starts causing safari to crash.
comment:6 Changed 16 years ago by
This is the revision that causes the packed code to crash safari:
http://dev.jquery.com/changeset/1595
1594 works.
I'm taking a look at the packer to see if there's a good reason for this to happen on those strings.
comment:7 Changed 16 years ago by
Applying this diff makes safari 1.3.2 not crash (however, nothing works correctly because all the selectors are broken):
Index: src/selector/selector.js
===================================================================
--- src/selector/selector.js (revision 2214)
+++ src/selector/selector.js (working copy)
@@ -61,9 +61,11 @@
// Match: :contains('foo')
/^(:)([\w-]+)\("?'?(.*?(\(.*?\))?[^(]*?)"?'?\)/,
+ jQuery.chars = "(?:[\\w\u0128-\uFFFF*_-]|\\\\.)"
+
// Match: :even, :last-chlid, #id, .class
new RegExp("^([:.#]*)(" +
- ( jQuery.chars = "(?:[\\w\u0128-\uFFFF*_-]|\\\\.)" ) + "+)")
+ ( jQuery.chars ) + "+)")
],
multiFilter: function( expr, elems, not ) {
If I simply replace all instances of jQuery.chars with the string that is assigned to it in the line above, jQuery loads in Firefox (and works) but crashes Safari 1.3.2.
comment:8 Changed 16 years ago by
I accidentally omitted a semicolon in the above diff. Adding the semicolon causes safari to crash and everything to work again, so it isn't the placement of that assignment inside the string concatenation that is causing the crash. It is the packing of something in that string (or so it appears).
comment:9 Changed 16 years ago by
I'm sorry for all the misleading comments on this ticket. I made a mistake during one of the first builds and copied the unpacked jquery.js from the source directory rather than the dist directory and thus assumed that the file was not crashing safari when it was unpacked in my simple test case of loading the library.
This is not the case. It crashes it compressed or uncompressed. The revision where the bug appears is still correct.
comment:10 Changed 16 years ago by
@jhenry: Please confirm this on unpacked version (from jresig's original 1.1.3 release email)... on line 849, if you remove (?:[
w\u0128-\uFFFF*_-]|
.) then Safari 1.3.2 does not crash.
comment:11 Changed 16 years ago by
More specifically, if I remove "\u0128-\uFFFF" from line 849, then Safari 1.3.2 does not crash when loading jquery-1.1.3.js. I don't understand enough about regular expressions or what this code does to recommend changes... hopefully this is enough...
comment:12 Changed 16 years ago by
Possible solution!
@jhenry, please confirm....
If you escape the slashes (so it becomes "
u0128-
uFFFF") then Safari 1.3.2 does not crash, AND it successfully parses IDs and classes and what not. However, Safari 2.0.4 does not understand this... it requires the single slash.
u0128-
uFFFF works in:
Safari 1.3.2
MacFF2, WinFF2
WinIE6, WinIE7
\u0128-\uFFFF works in: Safari 2.0.4 MacFF2, WinFF2 WinIE6, WinIE7
I'll let you guys figure out how to implement it... I don't want to have all the fun. :) Also note that "\u0128\uFFFF" (sans hyphen) does not crash Safari 1.3.2... but obviously does not have the intended result either. :)
comment:13 Changed 16 years ago by
The real problem is that Safari 2.0.4 doesn't understand
u0128 as a unicode character. Safari 2.0.4 requires \u0128 to match in a regular expression. This can be verified using the following test:
\u0041 == A: <span id="jsUnicodeResults2">Test Not Run</span><script type="text/javascript">document.getElementById("jsUnicodeResults2").innerHTML = ("\u0041" == "A");</script><br />
Regex "\u0041" matches "A": <span id="jsUnicodeResults4">Test Not Run</span><script type="text/javascript">var jsReTest2 = new RegExp("\u0041");document.getElementById("jsUnicodeResults4").innerHTML = (jsReTest2.exec("A") != null);
It would seem to me that doing browser version detection to slot in the appropriate character codes for Safari 2.0.4 would solve the problem, but it has to detect for Safari 2.0.4 rather than the others.
comment:14 Changed 16 years ago by
On getting in to work this morning and checking on my 10.3 box, I found that using the escaped slash in the sequence
u0128-
u0FFF doesn't crash safari 1.3.2, but it doesn't really work either (none of the selectors seem to be working to allow jquery to operate).
The closest I can get to working in Safari 1.3.2 by changing that one line of code is to remove the unicode sequences.
So code for a workaround might be:
if(safari) {
if(version < 2) {
unicode = "";
}
else {
unicode = "\u0128-\u0FFF";
}
}
else {
unicode = "\\u0128-\\u0FFF";
}
jQuery.chars = "(?:[\\w" + unicode + "*_-]|\\\\.)";
comment:15 Changed 16 years ago by
I tested Safari 1.3.2 yesterday with some simple selectors (#foo, .bar, :even) and those seemed to be working with the
u0128-
u0FFF code. Is there a test suite I should be using instead?
comment:16 Changed 16 years ago by
I was just doing quick tests by dropping the modified jquery into my development site and one of the applications on the page was a tabber. All the divs in the tabber were layered on top of each other in Safari 1.3.2 with the
u0128-
u0FFF code. With the unicode removed, the visibility on each of the divs was set correctly.
comment:17 Changed 16 years ago by
Darn! I knew it was too good to be true. My simple tests were too simple... they work even when I remove unicode characters!
What do lines 848 and 849 do? The "Match :even, :last-child, #id, .class" comment doesn't seem to encapsulate what that line really does, or am I missing something? The other way of asking this is: if we remove \u0128-\u0FFF what should break?
Obviously getting Safari 1.3.2 to not crash is a priority (especially now that this is an official release and out in the wild). But breaking Safari 1.3.2 in some way (by removing the unicode) needs to be noted. .... and I'm still hoping we can find a work around ....
comment:18 Changed 16 years ago by
These folks have run into the same problem: http://dev.rubyonrails.org/ticket/8332
Their solution was to replace "\u0128-\u0FFF" with "
s
S" ... the comment for their fix was "Replaced Unicode range with another any-character character class." I made the change in my local jquery-1.1.3.js, Safari 1.3.2 does not crash and jQuery seems to work.
Is "\u0128-\u0FFF" synonymous with "
s
S" ... are we losing/gaining anything? How can I test to make sure this pattern still works as intended?
comment:19 Changed 16 years ago by
Browsing through other project repositories, I've found definitive information that unicode ranges in Safari 1.3.2 will never work.
http://webkit.org/blog/32/webkit-fixes-in-safari-202-mac-os-x-1043/
Another project was reporting a similar trouble with safari crashing. They wanted to match any character and they chose to use the combination of spaces and not spaces \s\S. We can use "not spaces" here to accomplish the same thing.
http://dev.rubyonrails.org/attachment/ticket/8332/scriptfragment-no-unicode.diff
The code will then read something similar to:
jQuery.chars = "(?:[\\S&&[^DO_NOT_MATCH_THIS]]|\\\\.)";
I've confirmed that new RegExp("
S").exec("\u0129") returns true in safari which means that \u0129 is matching as not a space.
There are obviously characters that should explicitly not be matched, and those should be negated in the intersection of the regexp. John: can you weigh in here on what this pattern should *not* match so we can build an analogue without the unicode patterns? If we cannot do it without using unicode patterns, we should simply remove unicode support for any build less than Safari 2.0.2 as they're not supported in the regular expression engine for that browser.
comment:20 Changed 16 years ago by
bcherne: it appears as though we were working on similar lines at the same time. I just need to know what in the unicode range \u0001 to \u0128 we cannot match in this regular expression so that it can go in the intersection with \S.
comment:21 Changed 16 years ago by
Thanks for doing all the hard work on this, jhenry - I think the ideal solution is just going to be falling back to a less-powerful regexp in Safari 1.3. Something like:
jQuery.browser.safari && jQuery.browser.version < "2.0.0" ? "
w" : "Unicode Regexp"
I'll look into this later today, to see if I can find a better solution.
comment:22 Changed 16 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
Fixed in SVN rev [2233]. jhenry, let me know if this works for you, I plan on pushing it live later today.
comment:23 Changed 16 years ago by
| Milestone: | 1.1.3 → 1.1.4 |
|---|
comment:24 Changed 16 years ago by
| Resolution: | fixed |
|---|---|
| Status: | closed → reopened |
Hi, I've retested this on OS X 10.3.x with Safari 1.2 and 1.3 and this bug is still open, it still crashes the browser :( - please let me know if you have any more luck with this or if you'd like me to test.
comment:25 Changed 16 years ago by
@Ru, jhrenry - No one that I know has a copy of Safari 1.3 to test with, so please any help is appreciated. I committed the fix that jhenry recommended - if it's still crashing, then it's not related to what jhenry mentioned.
comment:26 Changed 16 years ago by
Safari uses an internal build number for the version number instead of something sane like "2.0.2". The build number for 2.0.2 is 416.12, so the test string should be something like:
jQuery.browser.safari && jQuery.browser.version < "416.12" ? "\\w" : "Unicode Regexp"
I will patch the version in svn and test it as soon as I have access to my 1.3.2 box.
comment:27 Changed 16 years ago by
@ john - Hey, I'm currently testing with browsercam and it reports the error. I'm trying to get remote access or physical access to the particular Mac build but its taking time :(
comment:28 Changed 16 years ago by
Attached is a patch against r2248 verified working in Safari 1.3.2. Note that "
w" isn't enough as the character groups must be the same as the other half of the regexp for things to work correctly.
Changed 16 years ago by
| Attachment: | safari_unicode_regexp.patch added |
|---|
comment:29 Changed 16 years ago by
Incidentally, you do not need Safari 1.3.2 (and OS10.3.9) to test for this bug. You can test in safari 2.0.0 available at http://www.michelf.com/projects/multi-safari/ on a machine that has OS10.4 installed. (This is because the bug was fixed in 2.0.2)
comment:30 Changed 16 years ago by
@jhenry - I applied your patch (SVN rev [2253]) and loaded it up in Safari 2.0.0, but it still crashes. Can you confirm this?
comment:31 Changed 16 years ago by
It's a problem with the revision number comparison. It's not crashing for me in 1.3.2 because it can interpret the revision number as a float (eg. xyz.w). The reason that it's crashing in 2.0.0 is because the revision is not a valid floating point number. It is 412.2.2. If you add a parseInt(), it works.
Attached is an updated patch that works with 2.0.0.
Changed 16 years ago by
| Attachment: | safari_unicode_regexp_2.patch added |
|---|
comment:32 Changed 16 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | reopened → closed |
Committed in SVN rev [2256] - works in Safari 2.0.0 for me - great work jhenry. I wanna try and fix up a couple other bugs then I may do a 1.1.3.2 release - or just go straight to 1.1.4.
In the meantime, you can grab the code from the jQuery nightlies: http://code.jquery.com/nightlies/jquery-nightly.js
I know that this will be very hard to diagnose without an OS10.3 machine on which to test (since Apple doesn't allow Safari 1.3.2 to run on OS10.4). If no one has an installation of OS10.3, let me know and I'll see if I can come up with a way to test the engine.