Reliance on showModalDialog breaks CKEditor plugin in Chrome

And.. how to fix ?

+1 how to fix

+1 how to fix ?

+1 I'm in the same boat.

I got uber lazy but this works, you will need to further the UI box per your needs. (ckeditor/plugin.js)

/**
 * @file Plugin for inserting video tags with video_filter
 */
(function ($) {
  CKEDITOR.plugins.add('video_filter', {

    requires : [],

    init: function(editor) {

      // Add Button
      editor.ui.addButton('video_filter', {
        label: 'Video filter',
        command: 'video_filter',
        icon: this.path + 'video_filter.png'
      });
      // Add Command
      /*editor.addCommand('video_filter', {
        exec : function () {
          var path = (Drupal.settings.video_filter.url.wysiwyg_ckeditor) ? Drupal.settings.video_filter.url.wysiwyg_ckeditor : Drupal.settings.video_filter.url.ckeditor
          var media = window.open(path, { 'opener' : window, 'editorname' : editor.name }, "dialogWidth:580px; dialogHeight:480px; center:yes; resizable:yes; help:no;");
        }
      });*/
      editor.addCommand( 'video_filter', new CKEDITOR.dialogCommand( 'video_filter' ) );
        /*--Add Video Filter--*/
        CKEDITOR.dialog.add( 'video_filter', function( editor )
        {
            return {
                title : 'Add Video',
                minWidth : 400,
                minHeight : 200,
                contents :
                    [
                        {
                            id : 'general',
                            label : 'Settings',
                            elements :
                                [
                                    {
                                        type : 'text',
                                        id : 'file_url',
                                        label : 'URL',
                                        validate : CKEDITOR.dialog.validate.notEmpty( 'The link must have a URL.' ),
                                        required : true,
                                        commit : function( data )
                                        {
                                            data.file_url = this.getValue();
                                        }
                                    },
                                    {
                                        type : 'text',
                                        id : 'width',
                                        label : 'Width',
                                        validate : CKEDITOR.dialog.validate.notEmpty( 'Insert Width' ),
                                        required : true,
                                        commit : function( data )
                                        {
                                            data.width = this.getValue();
                                        }
                                    },
                                    {
                                        type : 'text',
                                        id : 'height',
                                        label : 'Height',
                                        validate : CKEDITOR.dialog.validate.notEmpty( 'Insert Height' ),
                                        required : true,
                                        commit : function( data )
                                        {
                                            data.height = this.getValue();
                                        }
                                    }
                                ]
                        }
                    ],
                onOk : function()
                {
                    var dialog = this,
                        data = {},
                        link = editor.document.createElement( 'p' );
                    this.commitContent( data );

                    var str = '[video:' + data.file_url;
                    if (data.width) {
                        str += ' width:' + data.width;
                    }
                    if (data.height) {
                        str += ' height:' + data.height;
                    }
                    /*if (params.align) {
                        str += ' align:' + data.align;
                    }
                    if (params.autoplay) {
                        str += ' autoplay:' + data.autoplay;
                    }*/
                    str += ']';

                    link.setHtml( str );

                    editor.insertElement( link );
                }
            };

        });


      // Register an extra fucntion, this will be used in the popup.
      //editor._.video_filterFnNum = CKEDITOR.tools.addFunction(insert, editor);
    }

  });

  function insert(params, editor) {
    var selection = editor.getSelection(),
      ranges = selection.getRanges(),
      range,
      textNode;

    editor.fire('saveSnapshot');

    var str = '[video:' + params.file_url;
    if (params.width) {
      str += ' width:' + params.width;
    }
    if (params.height) {
      str += ' height:' + params.height;
    }
    if (params.align) {
      str += ' align:' + params.align;
    }
    if (params.autoplay) {
      str += ' autoplay:' + params.autoplay;
    }
    str += ']';

    for (var i = 0, len = ranges.length; i < len; i++) {
      range = ranges[i];
      range.deleteContents();

      textNode = CKEDITOR.dom.element.createFromHtml(str);
      range.insertNode(textNode);
    }

    range.moveToPosition(textNode, CKEDITOR.POSITION_AFTER_END);
    range.select();

    editor.fire('saveSnapshot');
  }

})(jQuery);

Comment #5 fixed it for me so I went about creating a patch for this issue. Thanks !

updating issue status

Thanks for creating that patch but I think we should respect the coding standards. To indent lines with 4 spaces is really crazy.

The patch in #6 replaces the old showModalDialog with a standard CKEditor modal, which functions but not as nice as the old modal. A proper fix to get the old modal working for Chrome 37+ (and Firefox 39+) is still needed.

Until that time I rearranged the fix in #6 and added the autoplay and align elements to it. It also checks if showModalDialog is defined, so it can use it when supported.

I reworked the path in #9, and added a specific callback for the instructions, so the can also be used in the ckeditor dialog.
It uses an html element with an iframe (ughhhh) to load the instructions, as depending on http://ckeditor.com/addon/iframedialog would be overhead.

I made the width an height fields optional, and removed the complex nested command calls from previous patches.
The behavior is now the same as the original popup.

The menu cache should be cleared to see the embedded instructions, otherwise you get a page not found error in the ckeditor dialog.

Maybe the popup should be removed completely in favor of the ckeditor dialog?

Confirm that #11 fixes this issue for my sites.

#11 Work fine for me too. Thanks!

I think that is very important bug, commit it, plz.

Patch works fine, please commit.

Attached patch #11 backported to 6.x-3.0 for those who want it.

We use this module in our distribution, it is not part of the core distribution but used as a dependency for a module using the apps module that does not support applying patches.
Please commit this patch.
https://www.drupal.org/node/2385087
Best regards

Patch at #11 also works for D6 version.

I have tested this patch and confirm it works. Please commit and roll a new release.

This should be added to the next release.

#11 works for me too but I think all labels should be translatable.

added related link:
https://www.drupal.org/node/1580354

Combined the autoplay fix from https://www.drupal.org/node/1430680#comment-6221378 since that fix also needs to be applied to the modal dialog code as well.

#23 (along with https://www.drupal.org/node/2488242#comment-9979941) worked for my Drupal 7 install. Thanks!

Are we getting a new release soon, including these patches?

@Jamesap: It would help if you (and others) could review the patch.

I think we should clean up the patch in order to follow the Drupal coding standards.

@joep.hendrix: The most recent patch was not updated to RTBC status, and now yannickoo has stated it (most likely the JS additions) needs to be updated to the Drupal coding standards, so it's not quite there yet.

Hi All. I updated the patch in 23 so that it hopefully conforms to the coding standards.
Cheers,
LoopDuplicate

#31 corrects the issue. Thanks!

#31 seems good!

patch #31 applied and everything works fine now in Chrome

It's working for me too. Change to RBTC, it's very important patch I think.

Ok, I have commited the patch in #31. I'm sorry this has taken so long, I'm not that active anymore, and it seems the maintainers I've added aren't either.

PS: In case you update and still get the error, flush your Chrome cache. :)