After installing the fresh dev version and running update, I got a javascript error that prevented the TinyMCE editor from showing.

There is a change in the settings JScript string from:

    "plugins": {
      "tinymce": {
        "native": [  ],
        "drupal": {
          "break": {
            "title": "Teaser break",
            "icon": "/sites/all/modules/wysiwyg/plugins/break/images/break.gif",
            "iconTitle": "Separate the teaser and body of this content",
            "css": "/sites/all/modules/wysiwyg/plugins/break/break.css"
          }
        }
      },

to

  	"plugins": {
  	  "tinymce": {
	      "native": [  ],
	      "drupal": {
	        "break": {
		        "title": [ "Teaser break", "Teaser break", "Teaser break", "Teaser break" ],
		        "icon": [ "/sites/all/modules/wysiwyg/plugins/break/images/break.gif", "/sites/all/modules/wysiwyg/plugins/break/images/break.gif", "/sites/all/modules/wysiwyg/plugins/break/images/break.gif", "/sites/all/modules/wysiwyg/plugins/break/images/break.gif" ],
		        "iconTitle": [ "Separate the teaser and body of this content", "Separate the teaser and body of this content", "Separate the teaser and body of this content", "Separate the teaser and body of this content" ],
		        "css": [ "/sites/all/modules/wysiwyg/plugins/break/break.css", "/sites/all/modules/wysiwyg/plugins/break/break.css", "/sites/all/modules/wysiwyg/plugins/break/break.css", "/sites/all/modules/wysiwyg/plugins/break/break.css" ]
		 		  }
 		    }
      },

due to the changes in wysiwyg.module.

After overwriting with the dev version of wysiwyg.module from 2nd March, the error went away.

Hope it helps.

CommentFileSizeAuthor
#4 wysiwyg-HEAD.plugin-loading-part-II.patch5.77 KBsun
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Alan D.’s picture

Note that this was due to changes between wysiwyg.module,v 1.23 (2009/02/22) and v1.24 (2009/03/03). CVS version numbers are more useful in finding old file versions than dev release dates

sun’s picture

If I understand those JSON dumps correctly, then the error is that configuration values for Drupal plugins are repeated (and nothing else)?

Alan D.’s picture

It appeared to be the only thing that could have caused the error. After reverting the latest wysiwyg.module file, the Drupal HEAD JavaScript settings string fragment in question went back to the strings and the JavaScript error went away, and the editor appeared.

It was a pure upgrade (unzip via overwrite) + update.php from March 2nd dev version, not a reinstall.

sun’s picture

Priority: Normal » Critical
Status: Active » Needs review
FileSize
5.77 KB

ok. The plugin loading patch totally screwed up the plugin loading.

Note to self: Don't trust TwoD's testing results. ;)

Attached patch should make it work again. However, I fear that we need to rewrite major parts of the plugin loading and processing, because the workaround of this patch sucks.

sun’s picture

Braindump before I leave to bed:

- Patch changes the storage of native/Drupal plugin settings to be per format instead of per editor. This is required, because both 'plugin settings callback' and 'proxy plugin settings callback' accept the wysiwyg profile as function argument.

- Until now, there is no plugin that implements different settings per format. However, that is a very valid use-case - at the very least for Drupal plugins provided by other Drupal modules. (e.g. Image Assist passing on some input format specific settings per format) Also, it would feel a bit inconsistent if this part of the API would suddenly work per-editor instead of per-format like everything else.

- If settings are the same for all formats, we are cluttering Drupal.settings with silly duplicated configuration settings.

- I wondered whether it would be possible to define "default plugin settings" globally, optionally overridden by the editor, optionally overridden by the format. In short: global -> editor -> format. The JS plugin initialization and loading would care for merging the overrides appropriately. This could either be done by blowing up the plugin API - or - through some magic in wysiwyg_add_plugin_settings(), which would use array_diff() and similar functions to automatically shift identical settings into the global settings. Weird idea, the latter might introduce more trouble than it solves.

TwoD’s picture

Status: Fixed » Needs review

*Looks away and whistles innocently*

Bug reproduced, I must have forgotten/missed something first time... sorry about that. With the patch it now works fine.

I agree with what Sun says in #5, the plugin API needs an overhaul. It's a bit confusing as it is, and we need to make sure plugins can be properly initialized, attached and detached in a logical way for atleast all of the currently supported editors before finalizing a design.

I'm not sure if the editor needs to override plugin settings though (if you're talking about user-configurable settings), as everything is done per input-format like you said before. Anyway, if we find a use case for it, then it should of course be allowed.

Let's start a new issue for plugin API redesign tho, and keep this issue focused on atleast temporarily fixing my blunder... =P

sun’s picture

Agreed. :)

@Alan D: Can you confirm that this patch fixes the bug?

Alan D.’s picture

It is all good. No JScript errors in sight (After fixing my own...)

Nice work guys

sun’s picture

Status: Needs review » Fixed

Thanks, committed to all branches.

Status: Needs review » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.