Bug 22927

Summary: Using spaces for indent in Ext/Javascript code causes all Javascript execution to fail silently
Product: WebKit Reporter: Brian <brian-webkit>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Critical CC: barraclough, brian-webkit, entwicklung, oliver, rik
Priority: P2    
Version: 525.x (Safari 3.2)   
Hardware: PC   
OS: Windows XP   
Attachments:
Description Flags
Uses spaces for indent and fails in safari 3.2.1/chrome 1.0
none
Working file which has spaces converted to tab via regexp for [ ]{4} to tab.
none
simple regular expression limit test none

Description Brian 2008-12-18 16:56:56 PST
I have some Javascript that uses Ext components.  The indenting was done with spaces.  Loading the document in Firefox/IE works properly; the code is valid and there are no syntax issues associated with it.  Both Webkit browsers Safari and Chrome failed to execute the Javascript and failed to throw an error.  Simply nothing happened.  In fact, putting a <script>alert('hello');</script> in the body didn't even execute.

It turns out we fixed the problem by converting spaces to tabs.  Nothing else.  Suddenly it worked.  Simply regexp'ing [ ]{4} to tab fixed the issue.  We can't understand it.

I'm attaching a diff between the two files.  I'm also including the broken.html and working.html files for your review.  There are going to be a couple of different lines between the HTML files because I generated one from our still-broken staging server and another from my working local copy but aside from some IDs and whatnot, they should be identical.  The key difference is the spaces vs. tabs for indents.
Comment 1 Brian 2008-12-18 16:57:38 PST
Created attachment 26134 [details]
Uses spaces for indent and fails in safari 3.2.1/chrome 1.0
Comment 2 Brian 2008-12-18 16:58:06 PST
Created attachment 26135 [details]
Working file which has spaces converted to tab via regexp for [ ]{4} to tab.
Comment 3 Brian 2008-12-18 16:59:23 PST
You can get the diff between a working and broken file (before and after my regexp to convert [ ]{4} to a tab) here:  http://www.ghidinelli.com/wp-content/uploads/2008/12/webkit-fail.patch
Comment 4 Anthony Ricaud 2008-12-19 01:46:48 PST
I can't reproduce this. The console is complaining about a missing Ext variable. It seems you haven't linked the Ext library in your examples. 
Comment 5 Oliver Hunt 2008-12-19 06:49:06 PST
Brian are you able to provide a live testcase?

I ahve tried to get the testcase up aat http://nerget.com/bugs/bug22927.html but failed, in both webkit and firefox.  

I've had to make a few changes to try to get Ext available to no avail, there's an exception because it's attempting to use document.body before the body element is defined.

Random note to future adventurers: while the testcase has an xhtml doctype it isn't xhtml 
Comment 6 Brian 2008-12-19 09:00:05 PST
This HTML file is loaded into a Panel in an Ext app.  There are several files and a fair amount of data required to make it work.  I do have a live example but I won't be easily able to have side-by-side working and failure.

Also, I don't want to publish all of the code to this app in a public space.  What is the preferred approach - I can work thru a fail/working version this morning if someone wants to capture some data?   My IM is massd on Y! or pukkasoft on AIM.  I will be in my office in about an hour and can work thru it this morning.
Comment 7 Brian 2008-12-19 09:02:13 PST
> Random note to future adventurers: while the testcase has an xhtml doctype it
> isn't xhtml 

No?  It validates at validator.w3.org.  It's missing an xhtml compatible comment around the javascript but that doesn't break the document; it only generates warnings.

http://validator.w3.org/check?uri=https%3A%2F%2Fbugs.webkit.org%2Fattachment.cgi%3Fid%3D26135&charset=%28detect+automatically%29&doctype=Inline&group=0

Comment 8 Oliver Hunt 2008-12-19 12:52:45 PST
By having an the extension .html, the server sends it as text/html which triggers quirks mode (in most browsers due almost exclusively to this being IEs behaviour), if you rename to .xhtml (so it gets sent as application/xml+xhtml it fails because the scripts need to be in CDATA sections.

Anyhoo, If you have a page that can demonstrate the problem, but you wish it to remain private you can file a bug at bugreporter.apple.com, which i'm fairly sure will give you a radar #, then paste the radar in this bug, and i'll get it (hopefully) to us, and then associate it with this bug.

That way your testcase can be kept out of "public" 
Comment 9 Brian 2008-12-19 13:40:02 PST
It will take me some time to put together a test case that is static that I can deliver (this is a 3-pane Ext app with a lot of dynamic loading going on).  We were able to manipulate the file so that adding a single additional space to the file would break Safari/Chrome and removing that space would make it work.  It may actually be more file size related than space/tab although that was the solution that fixed the behavior.

I will work on a test case over the holidays and return here.
Comment 10 Oliver Hunt 2008-12-19 14:11:23 PST
Thanks Brian!

Oh, quick question -- Does the Wed Inspector console report an error, and if so what?
Comment 11 Brian 2008-12-19 15:25:24 PST
There are no errors reported - in fact no Javascript executes when this happens.  I was able to modify the <body> of the HTML to something like:

<body>
HELLO
<script>alert('hello!');</script>
</body>

And the HELLO would show up in the Ext panel - proving that the content was loading - but the alert would not fire.  Totally bizarre.  Both my computer and my contractor's computer showed the same behavior in all four browsers (working in FF/IE, failing in Safari/Chrome).
Comment 12 ES2000 2009-01-29 02:59:09 PST
I've experienced a similar bug. Further testing shows that the regular expression used by extjs to determine the javascript code is the problem. Here is the regular expression:

(?:<script([^>]*)?>)((\n|\r|.)*?)(?:<\/script>)

Webkits RegExp implementation find no match if the part between the script tags is too long. I will add a simple test file that shows the problem.
Comment 13 ES2000 2009-01-29 03:02:18 PST
Created attachment 27143 [details]
simple regular expression limit test
Comment 14 Brian 2009-03-22 18:08:57 PDT
I'm looking at trying to recreate my situation with a static test case.  However, it appears as though a solid test has been submitted which demonstrates the issue.  It will take a fairly large amount of time that I don't wish to spend if the submitted test by ES2000 is enough.  Can anyone comment?
Comment 15 Oliver Hunt 2009-03-22 18:31:17 PDT
We've recently increased the regex size limit -- does this bug still repro?
Comment 16 ES2000 2009-03-25 08:44:29 PDT
I have tested it again with the following versions:

Safari 4 Public Beta (528.16) on Windows: bug still exist
Webkit Nightly Build r41944 on MacOS: bug seems to be solved
Google Chrome (2.0.171.0) on Windows: bug seems to be solved

So will the final release of Safari 4 get the fix that is in the nightly version?
Comment 17 Oliver Hunt 2009-03-25 12:51:10 PDT
Could you please provide a testcase that fails? the regex limit test appears to work fine, the two big tests do not run because they are incomplete
Comment 18 ES2000 2009-06-09 03:10:31 PDT
Tested my case again in the final version of Safari 4 under Windows and it seems to be fixed. Maybe Brian could comment again if his cases are also fixed so this bug could be closed.
Comment 19 Brian 2009-06-09 13:15:41 PDT
I don't have the facility to test the broken case any longer, sorry. :(
Comment 20 Gavin Barraclough 2011-09-08 18:13:04 PDT
The attached test cases seem to work & the limits have been raised, based on comments looks like this should now be fixed.  Please do reopen if this still repros.