#18 ✓resolved
Adam B

TinyMCE::Configuration#to_json adds a trailing comma (breaks IE6)

Reported by Adam B | February 19th, 2010 @ 07:55 AM

Installed 2010-02-18.

using <%= include_tiny_mce_if_need %> with no custom configuration options renders the call to tinyMCE.init like this:

<script src="/javascripts/tiny_mce/tiny_mce_src.js?1266514020" type="text/javascript"></script>
<script type="text/javascript">
//<![CDATA[
tinyMCE.init({
editor_selector : 'mceEditor',
language : 'en',
mode : 'textareas',
theme : 'simple',

});
//]]>
</script>

Which breaks IE6 due to the trailing comma inside the object that is passed to tinyMCE.init. I fixed it for myself by replacing this:

json_options += @raw_options unless @raw_options.blank?
json_options << raw_options unless raw_options.blank?

with this:

json_options += @raw_options unless (@raw_options.blank? || (@raw_options.size == 1 && @raw_options.first.chomp === ''))
json_options << raw_options unless (raw_options.blank? || raw_options.nil? || raw_options.empty?)

The problem is that @raw_options is [''], so in the code's original form, the empty string is appended to json_options, and so the extra comma is always inserted.

Comments and changes to this ticket

  • Kieran P

    Kieran P February 19th, 2010 @ 08:58 AM

    Hey Adam,

    There is a fix for this at http://github.com/mooman/tiny_mce/commit/34cc321ea343789870cd432677...

    I'm awaiting a testcase from the author of the patch before pulling it in.

    If you want to help out and make a test that duplicates the error, that'd be good.

    Regards
    Kieran

  • Adam B

    Adam B February 19th, 2010 @ 09:17 AM

    test/test_helper.rb makes a call out to a file that's not in the git repo:

    require File.expand_path(File.dirname(__FILE__) + "/../../../../test/test_helper")
    

    Is there anything significant in that file? I'm having trouble running the tests (failed "require"s, uninitialized constants). Which version of ActionView is the test suite looking for?

    • Adam
  • Kieran P

    Kieran P February 19th, 2010 @ 09:22 AM

    The tests are designed to be run within a Rails app's plugin dir for testing with AV and AC. The requires pulls in Rails.root/tests/test_helper. It would be nice if the Rails controllers were mocked though, so we could run them wherever the source code is.

  • Adam B

    Adam B February 19th, 2010 @ 10:18 AM

    Looks like it may be a pre-Rails 2.3 bug. I can't reproduce in 2.3.5 and I can't test in 2.1.2 (ActiveSupport 2.1.2 doesn't define the test "name of test" do; ...; end idiom). Making the test_suite runnable in 2.1.2 looks like more work than I can justify right now, and probably isn't worth it in the long run.

    The patch you linked to works fine and is cleaner than my original suggestion. I'm in prototype mode right now, anyways. If we end up pushing tiny_mce to production, I'll take the time to patch and figure out my testing woes so I won't get bitten in the future.

    Thanks for your help,
    Adam

  • Kieran P

    Kieran P February 27th, 2010 @ 10:05 PM

    • State changed from “new” to “resolved”

    A fix has been pulled into the master, and a bit of refactoring. It should fix this issue for this ticket.

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป

A Ruby on Rails plugin that allows easy implementation of the TinyMCE editor into your applications.

People watching this ticket

Pages