TinyMCE 3.2.7, clicking page break button won't insert page break (does nothing) in full screen mode, works as expected when not in full screen mode.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

TwoD’s picture

Title: Full Screen - Page Break » Inserting content in fullscreen TinyMCE
Version: 6.x-2.0 » 6.x-2.x-dev
Status: Active » Needs review
FileSize
1.4 KB

TinyMCE creates a completely different editor for fullscreen mode and does not relay changes made to the original editor to the new one. We explicitly state which instance we want the inserted content to go to, so we must check to see if there's an active fullscreen editor and if it was created based on the correct instance.
Here's a patch to 6.x-2.x-dev to fix that. It can be applied to 2.0 as well.

Thanks for finding and reporting this issue.

Internetter’s picture

Please look at the functions openDialog() and closeDialog(). These should also get the "fullscreen" check. Perhaps it is possible to set this.field right, if tinymce is in fullscreen-mode.

Here are fragment, sorry not complete patch. ;-(

  openDialog: function(dialog, params) {
  	var instanceId = tinyMCE.activeEditor.id == 'mce_fullscreen' && tinyMCE.activeEditor.getParam('fullscreen_editor_id') == this.field ? 'mce_fullscreen' : this.field;
    var editor = tinyMCE.get(instanceId);
    editor.windowManager.open({
      file: dialog.url + '/' + instanceId,
      width: dialog.width,
      height: dialog.height,
      inline: 1
    }, params);
  },

  closeDialog: function(dialog) {
  	var instanceId = tinyMCE.activeEditor.id == 'mce_fullscreen' && tinyMCE.activeEditor.getParam('fullscreen_editor_id') == this.field ? 'mce_fullscreen' : this.field;
    var editor = tinyMCE.get(instanceId);
    editor.windowManager.close(dialog);
  },

bye Martin

TwoD’s picture

Updated patch, adds a new isFullscreen() method to the tinyMCE implementation.
I hope the ternary operators are clear enough.

sgabe’s picture

I created a wysiwyg plugin based on the API documentation and the break plugin example in the module and discovered this bug. The break plugin didn't work in full screen mode, my plugin neither. Patch in #1 solved the problem, but the latest patch in #3 didn't work at all. The whole editor disappeared after the patch...

TwoD’s picture

I re-tested the #3 patch with the latest -dev snapshot and TinyMCE 3.3b2 and it applies correctly (with a 3-row offset due to the recently committed TinyMCE 3.3 support patch) and I'm able to use Teaser Break in fullscreen mode.

Do you see any JavaScript errors?
If not, could you perhaps try setting a few brekpoints with Firebug for FF to see if the attach() method in wysiwyg/editors/js/tinymce-3.js i executed correctly?

sgabe’s picture

I checked the latest 6.x-dev from CVS, applied your patch again and now it works fine. Think I've mistaken. Thanks!

JGO’s picture

Indeed, everything is working fine thanks to this patch!!

TwoD’s picture

Status: Needs review » Reviewed & tested by the community

I think #6 and #7 makes this RTBC, doesn't it?

Aside from the trailing space after isFullscreen(), I can't find any syntax bloopers either.

sgabe’s picture

I can confirm this as RTBC.

TwoD’s picture

Status: Reviewed & tested by the community » Fixed

Patch in #3 committed to all branches.

The fix will be in -dev snapshots soon and will be part of the next stable release.

Thank you all for reviewing, testing and commenting!

sun’s picture

+++ editors/js/tinymce-3.js	19 Jan 2010 23:37:57 -0000
@@ -106,7 +106,9 @@ Drupal.wysiwyg.editor.instance.tinymce =
+            var instanceId = ed.id == 'mce_fullscreen' ? ed.getParam('fullscreen_editor_id') : ed.id;

@@ -163,9 +165,10 @@ Drupal.wysiwyg.editor.instance.tinymce =
+    var instanceId = this.isFullscreen() ? 'mce_fullscreen' : this.field;

@@ -173,7 +176,8 @@ Drupal.wysiwyg.editor.instance.tinymce =
+    var instanceId = this.isFullscreen() ? 'mce_fullscreen' : this.field;

@@ -208,7 +212,13 @@ Drupal.wysiwyg.editor.instance.tinymce =
+    var instanceId = this.isFullscreen() ? 'mce_fullscreen' : this.field;

Minor note (mainly for TwoD): Whenever we do inline-ifs (or more officially: when using the ternary operator), then the entire expression should be wrapped in parenthesis; e.g.:

$foo = ($bar ? 'baz' : 'beer');

Doing so helps to prevent unexpected bugs and heavily clarifies the code for people that happen to read it. Thanks! :)

Powered by Dreditor.

TwoD’s picture

Oh, right, thanks!

Status: Fixed » Closed (fixed)

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