Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #9663 (new defect)

Opened 1 year ago

Last modified 5 months ago

[PATCH][TEST] stripScripts/extractScripts don't work in Safari if non-ASCII characters are used in scripts

Reported by: Alexey Proskuryakov Assigned to: Tobie
Priority: normal Milestone: 2.x
Component: Prototype Version: edge
Severity: normal Keywords: 1.6.0.3
Cc:

Description

Due to a bug in Safari/WebKit, [\s\S] regexp pattern doesn't match non-ASCII characters. Since this pattern is used in Prototype.ScriptFragment, this causes compatibility problems, see <http://bugs.webkit.org/show_bug.cgi?id=15224>.

I think that replacing \S with \s should work as a workaround, but I haven't tested that.

This bug is present in shipping versions of WebKit, as well as in nightly builds. Using Prototype.js version 1.5.1.1.

Attachments

extractScripts_test.diff (1.3 kB) - added by jdalton on 03/26/08 16:12:45.
unit test for exctractScripts, tests with non ascii characters and 7000+ char length
scriptFragment_safari_fix.diff (439 bytes) - added by jdalton on 03/26/08 16:13:58.
implemented the new regexp for scriptFragment tested on safari 2.0.4

Change History

09/24/07 17:30:26 changed by Alexey Proskuryakov

  • owner changed from core to sam.
  • component changed from ActiveRecord to Prototype.

(follow-up: ↓ 3 ) 09/24/07 22:07:56 changed by Tobie

  • owner changed from sam to Tobie.

Unfortunately, your suggested solution got mangled up. Could you please post it again ?

For info, we're already using this stupid regexp instead of a more logical one to handle yet another Safari bug, so we'll see what we can do.

(in reply to: ↑ 2 ) 09/25/07 04:32:31 changed by Alexey Proskuryakov

Replying to Tobie:

I was just repeating a suggestion from PCRE developers: <http://bugs.exim.org/show_bug.cgi?id=603>. Apparently, it got mangled in their system, too - but my understanding is that non-whitespace character class can be replaced with a negated whitespace one.

For info, we're already using this stupid regexp instead of a more logical one to handle yet another Safari bug

Was it filed in WebKit bugzilla?

09/25/07 06:30:13 changed by Tobie

Was it filed in WebKit bugzilla?

No as it's been dealt with in the nightlies for a while now.

09/25/07 06:42:33 changed by Tobie

Any suggestion on how to fix this is welcomed, knowing that this /(\s\S)+/ crashes Safari on strings with roughly 7'000 characters.

09/25/07 19:31:26 changed by Alexey Proskuryakov

This is the only variant I could find that worked and didn't crash Safari 2: '<script[^>]*>([^\\x00]*?)<\/script>' - it assumes that there are no null characters in the script. But I'm not a regexp guru by any means.

Another option could be to move part of the logic to JS, matching beginning and trailing <script> tags in a loop. I'm not sure if this would have a measurable negative impact on performance.

09/25/07 19:37:47 changed by Tobie

Does that work in safari 1.3 ?

09/26/07 04:26:59 changed by Alexey Proskuryakov

Yes, it does.

03/26/08 16:12:45 changed by jdalton

  • attachment extractScripts_test.diff added.

unit test for exctractScripts, tests with non ascii characters and 7000+ char length

03/26/08 16:13:58 changed by jdalton

  • attachment scriptFragment_safari_fix.diff added.

implemented the new regexp for scriptFragment tested on safari 2.0.4

03/26/08 16:14:36 changed by jdalton

  • summary changed from stripScripts/extractScripts don't work in Safari if non-ASCII characters are used in scripts to [PATCH][TEST] stripScripts/extractScripts don't work in Safari if non-ASCII characters are used in scripts.

03/29/08 07:50:18 changed by jdalton

  • keywords set to 1.6.0.3.

03/29/08 08:20:40 changed by jdalton

the following ticket has the patch provided here rolled into it http://dev.rubyonrails.org/ticket/9599