Bug 54978 - REGRESSION (r72489): TinyMCE not working in nightlies
Summary: REGRESSION (r72489): TinyMCE not working in nightlies
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P1 Normal
Assignee: Michael Saboff
URL: http://tinymce.moxiecode.com/tryit/fu...
Keywords: InRadar, NeedsReduction, Regression
: 52106 55520 (view as bug list)
Depends on: 46719
Blocks:
  Show dependency treegraph
 
Reported: 2011-02-22 11:57 PST by Alexander Romanovich
Modified: 2011-03-03 23:00 PST (History)
15 users (show)

See Also:


Attachments
Patch to temporarily disable begin characters optimization (1.47 KB, patch)
2011-03-01 10:55 PST, Michael Saboff
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Romanovich 2011-02-22 11:57:59 PST
I don't know when this started, but it seems to have broken some time ago. The attached TinyMCE example page (from their site) works in Safari release but not in WebKit.

In Safari, the content inside the box is rendered. In the nightlies, the raw HTML source is displayed. My concern of course is making sure that the breakage doesn't find its way into a Safari release.
Comment 1 Alexey Proskuryakov 2011-02-22 17:26:07 PST
<rdar://problem/9039914>
Comment 2 Andy Estes 2011-02-22 18:05:57 PST
This is due to <http://trac.webkit.org/changeset/72489>.
Comment 3 Ryosuke Niwa 2011-02-27 14:57:23 PST
Could you make a reduction for this bug?  In particular, it's not clear what javascript isn't ran properly.  It'll be much easier for us to debug if we had 10 line html/javascript that demonstrates the bug.  Also, this bug might be a duplicate of https://bugs.webkit.org/show_bug.cgi?id=52106.
Comment 4 Alexander Romanovich 2011-02-28 07:33:39 PST
This is the simplest test case I know how to make, not being associated with TinyMCE in any way:

http://www.sirensclef.com/webkit/tinymce/

It will render the contents of the textarea in Safari release, but show (broken) source in nightlies. If you need further reductions, you could contact the TinyMCE folks directly, so that they know about this bug and can help diagnose it further.
Comment 5 Michael Saboff 2011-03-01 09:57:35 PST
This bug is caused by improper handling of the regular expression /<(?:(?:!--([\w\W]*?)-->)|(?:!\[CDATA\[([\w\W]*?)\]\]>)|(?:!DOCTYPE([\w\W]*?)>)|(?:\?([^\s\/<>]+) ?([\w\W]*?)[?/]>)|(?:\/([^>]+)>)|(?:([^\s\/<>]+)\s*((?:[^"'>]+(?:(?:"[^"]*")|(?:'[^']*')|[^>]*))*)>))/

This regular expression appears to be looking for the contents inside <> HTML constructs.

The failing regular expression has been reduced to: /<((ABC>)|(\/([^>]+)>)|(([^>]+)>))/
with the (ABC>) added to make it fail.

The bug appears to be an issue with the "Beginning Characters" optimization added that seems to have a problem in the YARR interpreter.  When the beginning characters optimization is disabled, the failing page operates correctly.  The reason the (ABC>) was added to the above expression was that without that alternative, the beginning characters optimization worked correctly.  It seems that the bug has to do with how far down the alternatives the begin characters setup code analyzes when creating the data for the  optimization to use at runtime.

I will create a patch that will disable the beginning characters optimization and then create a new defect to address the beginning characters bug.
Comment 6 Michael Saboff 2011-03-01 10:55:41 PST
Created attachment 84251 [details]
Patch to temporarily disable begin characters optimization
Comment 7 Darin Adler 2011-03-01 11:48:54 PST
Comment on attachment 84251 [details]
Patch to temporarily disable begin characters optimization

OK.
Comment 8 Michael Saboff 2011-03-01 11:54:58 PST
Committed r80018: <http://trac.webkit.org/changeset/80018>
Comment 9 Alexey Proskuryakov 2011-03-01 12:14:17 PST
No regression test?
Comment 10 Darin Adler 2011-03-01 12:15:59 PST
(In reply to comment #9)
> No regression test?

I think the intent is to add the test when we turn the optimization back on, but I agree that it would be good to add the test now.
Comment 11 Michael Saboff 2011-03-02 09:15:31 PST
*** Bug 55520 has been marked as a duplicate of this bug. ***
Comment 12 Ryosuke Niwa 2011-03-03 23:00:03 PST
*** Bug 52106 has been marked as a duplicate of this bug. ***