Opened 10 years ago

Last modified 7 years ago

#12583 confirmed Bug

Certain edit operations destroy the protected structure of a widget

Reported by: jhub Owned by:
Priority: Normal Milestone:
Component: General Version: 4.3
Keywords: Blink Webkit Cc:

Description

Reproduce with the help of the simplebox widget:

Click the toolbar button to insert a simplebox.

In the second of the two editable boxes of the widget, set the cursor somewhere in the middle of the word "Content...".

Use SHIFT+Cursorkeys to move the cursor up into the first of the editable boxes, somewhere in the middle of the word "Title", so that you have a selected text area that selects the end of the word "Title" and the beginning of the word "Content...".

Press DELETE to delete the selected text.

The result is as expected: The remaining text is now in the first box (the title box), but the content box, which is now empty, is still there, i.e. you can again insert new text into the content box.

Now make the following slight modification to the simplebox plugin:

Change the template so that the title box (the one with class="simplebox-title") is a div instead of a h2, like so:

template:
    '<div class="simplebox">' +
        '<div class="simplebox-title">Title</div>' +
        '<div class="simplebox-content"><p>Content...</p></div>' +
    '</div>',

(Also for completeness sake, change the "allowedContent" so that it says "div(!simplebox-title)" instead of "h2(!simplebox-title)".)

Reload the editor and repeat the above experiment.

You will see that when you press DELETE to delete the highlighted text, that the text is deleted as expected, but the second editable box (the div with class="simplebox-content") is also deleted, i.e. a protected part of the widget is damaged. You can no longer add new text into the "content" field of the widget since it no longer exists.

Attachments (1)

simpleboxmod.zip (7.2 KB) - added by Jakub Ś 9 years ago.

Download all attachments as: .zip

Change History (12)

Changed 9 years ago by Jakub Ś

Attachment: simpleboxmod.zip added

comment:1 Changed 9 years ago by Jakub Ś

Keywords: Blink Webkit added
Status: newconfirmed
Version: 4.4.54.3

Problem can be reproduced from CKEditor 4.3 in Blink and Webkit browsers.

It seems that it isn't possible to select content from different editables with mouse however SHIFT+ARROW has no problems with passing editables' borders. This still isn't a problem but...

When you have different elements for editables, only text inside them will be removed using the above method. When on the other hand you have same elements used for editables then bottom one will be removed together with text.

To reproduce:

  1. use attached plugin in sample
  2. Select parts of text from both editables using SHIFT+ARROW
  3. Press Delete or Backspace.

Result: bottom editable will be removed.

comment:2 Changed 8 years ago by kkrzton

I believe there is a similar case to the one above. Using simple plugin like this:

CKEDITOR.plugins.add('testwidget', {
  "requires": ['widget'],
  "init": function(editor) {
    editor.ui.addButton('testwidget', {
      "label": 'testwidget',
      "command": 'testwidget',
    });

    editor.widgets.add('testwidget', {
      "template": '<div class="testwidget">' +
                    '<div class="one">Triple click & delete</div>' +
                    '<div class="two">Editable</div>' +
                  '</div>',
      "editables": {
        "one": {
          "selector": '.one',
        },
        "two": {
          "selector": '.two',
        },
      },
      "allowedContent": 'div(!testwidget); div(!one); div(!two)',
      "upcast": function(element) {
        return element.name == 'div';
      }
    });
  }
});

CKEDITOR.replace('editor', {
  extraPlugins: 'testwidget',
  toolbar: [['Source'],['Bold','Italic'],['testwidget']],
});

and following the scenario:

  1. Click the widget button (blank) to insert a widget.
  2. Triple click (fast) in the first field.
  3. Press Delete key.
    • The first field is completely removed.

also deletes protected part of the widget which should not be deleted.

comment:3 Changed 8 years ago by Piet

Working on a client solution that they can edit content in (bootstrap) columns, but not destroy the column setup.

In my example, using a widget with bootstrap 2 columns:

            editables: {
                col1: {
                    selector: '.col-1',
              },
                col2: {
                    selector: '.col-2',
                  }
            },
            template:

		'<div class="row two-col bstwocolumn">' +
                '<div class="col-md-6 col-1"><p>Content</p></div>' +
                '<div class="col-md-6 col-2"><p>Content</p></div>' +
                '</div>',

And indeed bumping into the same issue, in a webkit browser I can actually double-click and delete items in col1, which in turn deletes entire col2 setup.

Have a look here for a visual http://blueocean.clarify-it.com/d/6veekf

So hoping for a solution to this problem as we really like to avoid our customers to break the column structure.

Anyone?

Thanks in advance!

comment:4 Changed 7 years ago by Brad Balfour

Any idea when this bug might be fixed?

comment:5 Changed 7 years ago by Jakub Ś

Related issue #14353.

comment:6 Changed 7 years ago by Jakub Ś

This is something we would like to work on in the future but currently have no ETA for.

comment:7 Changed 7 years ago by Brad Balfour

Ok. We managed to work around this by trapping the change event in the widget and looking to see if the change deleted one of the div's in the widget template. And, if so we re-insert it into the DOM. A hack, but it works. Essentially a self sealing widget.

comment:8 in reply to:  7 Changed 7 years ago by Piet

Replying to bbalfour:

Ok. We managed to work around this by trapping the change event in the widget and looking to see if the change deleted one of the div's in the widget template. And, if so we re-insert it into the DOM. A hack, but it works. Essentially a self sealing widget.

Any chance you can supply the source code of how you have done this?

comment:9 Changed 7 years ago by Brad Balfour

Here you go...

The context of this is additions/enhancements to the image2 widget. Essentially we always have both a caption and credit along with the image. and they have specific data attributes/values and classes.

The failure of the findOne() to return a real element and instead return null is how we detect that the widget template is broken.

Here's the code:

    widget.on("data", function(evt) { // jshint ignore:line
        //augment the data definition with ours that changes our extra data

        /** In attempting to delete the contents of the caption, the user
         * can hit a CKEditor + Chrome bug where the actually cause the DOM
         * to delete entire caption field in the image widget and move the credit's
         * contents into the caption field.
         *
         * NOTE: we cannot make this bug happen on the Credit field deletion
         * so that case is not handled here
         *
         * This is a CKEditor bug. It has been reported in:
           * http://dev.ckeditor.com/ticket/12583
           * http://dev.ckeditor.com/ticket/14354
           * http://dev.ckeditor.com/ticket/14353

         * According to CKEditor it is Chrome specific as that is the only browser
         * where you can make a selection across content editable boundaries.
         * And then, only by a triple click and not by a shift click approach.
         */
        var caption = this.element.findOne("div.news-figure-caption-text");
        var credit = this.element.findOne("div.news-figure-credit");
        var figcaption = this.element.findOne("figcaption");
        if (caption && !credit && this.parts.avmmCredit) {
            var realCreditText = caption.getText();
            this.parts.avmmCredit.setText(realCreditText);
            figcaption.append(this.parts.avmmCredit);
            this.data.avmmCredit = realCreditText;
            caption.setText("");
            this.data.caption = "";
        }

comment:10 in reply to:  9 Changed 7 years ago by Piet

Thank you very much!

Is it possible to include the actual widget code to see the relation between your code and the widget, we'd be using in it a different way and some parts of the code shown do not make sense to me without seeing the widget (not your fault, more my lack of programming experience ;)

comment:11 Changed 7 years ago by Brad Balfour

The widget code is the standard CKEditor image2 widget code. We have defined the lines above in a separate file so that we can tweak image2 and not have our code broken/overwritten by any future changes to image2. The "missing" lines are only:

        this.editor.on("pluginsLoaded", function(evt) {
            //must override init() and data() on instanceCreated once we have a widget
            //widgetDefinition is too soon and we don't have the widget to bind to its events
            evt.editor.widgets.on("instanceCreated", imageWidgetAdditions.modifyImageWidgetInitData);
        });

and these lines just above the code from my last response:

imageWidgetAdditions.modifyImageWidgetInitData = function(evt) {
    var widget = evt.data;

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy