summernote / summernote

Super simple WYSIWYG editor
https://summernote.org
MIT License
11.53k stars 2.25k forks source link

Intermittent issue with copy paste content in reverse order #1964

Closed esuhendra closed 4 years ago

esuhendra commented 8 years ago

Hi there,

Been having intermittent issues with copy pasting text from either Word or Notepad. Several times the last line of the content ended up at the beginning of the section being pasted. At first I thought this is because of Word formatting, but sometimes it happens when copying content from Notepad. Has anybody experience this before?

It is intermittently happening..

steps to reproduce

  1. Copy text content from either Word or Notepad.
  2. Paste into summernote.

    browser version and os version

What is your browser and OS? Windows 7 and 10 with IE11.

Any help is greatly appreciated.

avif commented 8 years ago

Got a repeating scenario for this (with IE11 & Win8.1):

  1. Copy some html content.
  2. Paste it in summernote with right click > paste.
  3. Delete the pasted content.
  4. Paste it again with CTRL+V.

Not sure if this is the only scenario, but at least this one works.

MS37 commented 7 years ago

FYI - We're also getting this same bug w/ slightly different repro steps. Paste will reverse the order of any paragraphs if the cursor is at the start of a line w/ text (middle or end of the line works fine).

Steps:

  1. Open http://summernote.org/ in IE
  2. Copy multiple lines/paragraphs of text from notepad or word
  3. Type "Test" into the 1st line in the summernote control
  4. Place the cursor to the left of the word "Test"
  5. CTRL+V to paste
  6. Bug = It pastes the lines of text in the reverse order

For example, copy this text: 1 one 2 two 3 three

It'll paste like this: 3 three2 two 1 one Test

DaniilVeriga commented 7 years ago

We have faced the same issue.

Dear Summernote Team, @easylogic, @hackerwins, @insanehong, @lqez, @outsideris, @seapy, are anybody aware of this quite severe issue? Do you think it is a chance to make the great product even greater?;)

marcinkoziej commented 6 years ago

Any updates on this issue?

lqez commented 6 years ago

Sorry for letting it alone. It looks like there are several duplicated like https://github.com/summernote/summernote/issues/2519.

fnccdna commented 6 years ago

Hello, Do we have this fix identified or scheduled?

Robertkeli commented 6 years ago

Haaah,,,I thought I faced the issue because I used the CleanPaste plugin,,,,

Any fix?

marcinkoziej commented 6 years ago

I also have this issue, the simplest case to reproduce was to start with one later a:

<div class="note-editable" contenteditable="true" role="textbox" aria-multiline="true" style="height: 450px;">a</div>

Then place the cursor before a and press ctrl-v to paste three lines from a word file (libre office writer):

a
b
c

The result is:

<div class="note-editable" contenteditable="true" role="textbox" aria-multiline="true" style="height: 450px;"><p>  </p><p>c</p><p> </p><p>b</p><p> </p><p>a</p><p>                     a</p></div>

so:

c
b
a
a

The order of lines is reversed.

I think this is related to:

    WrappedRange.prototype.pasteHTML = function (markup) { 
    (...)
        return childNodes.reverse().map(function (childNode) {
            return rng.insertNode(childNode);
        }).reverse();

Here we insertNode's in reverse order, apparently assuming that after pasting one line, the next line is pasted before the first - so pasting in reverse will eventually result in correct order. But this is not the case. This works if I put the cursors after letter a and paste, however.

Probably insertNode is also to be invetigated:

    WrappedRange.prototype.insertNode = function (node) {
        var rng = this.wrapBodyInlineWithPara().deleteContents();
        var info = dom.splitPoint(rng.getStartPoint(), dom.isInline(node));
        if (info.rightNode) {
            info.rightNode.parentNode.insertBefore(node, info.rightNode);
        }
        else {
            info.container.appendChild(node);
        }
        return node;
    };

The reverse problem can happen when I try to paste placing the cursor in different places, but also when I select text I want to replace, and then paste.

I am using summernote-cleaner.js, which calls this primitives to paste final, cleaned HTML. I can confirm that summernote-cleaner.js is passing a correct value, so it's not to blame here.

os-tempoconsulting commented 6 years ago

A proposal to solve this problem :

File: src/js/base/core/range.js

pasteHTML(markup) {
  const contentsContainer = $('<div></div>').html(markup)[0];
  let childNodes = lists.from(contentsContainer.childNodes);

  const rng = this.wrapBodyInlineWithPara().deleteContents();
  if (rng.so > 0) {
    childNodes = childNodes.reverse()
  }
  childNodes = childNodes.map(function(childNode) {
    return rng.insertNode(childNode);
  })
  if (rng.so > 0) {
    childNodes = childNodes.reverse()
  }
  return childNodes
}
DaniilVeriga commented 6 years ago

The issue is still reproducible even with the last fix applied... More details is in this comment: https://github.com/summernote/summernote/pull/2796#issuecomment-391645784

paulwilton commented 6 years ago

I confirm the splitPoint function is causing paste problems, when invoked via pasteHtml command

function splitPoint(point, isInline) {
    // find splitRoot, container
    //  - inline: splitRoot is a child of paragraph
    //  - block: splitRoot is a child of bodyContainer
    var pred = isInline ? isPara : isBodyContainer;
    var ancestors = listAncestor(point.node, pred);
    var topAncestor = lists.last(ancestors) || point.node;
    var splitRoot, container;
    if (pred(topAncestor)) {
        splitRoot = ancestors[ancestors.length - 2];
        container = topAncestor;
    }
    else {
        splitRoot = topAncestor;
        container = splitRoot.parentNode;
    }
    // if splitRoot is exists, split with splitTree
    var pivot = splitRoot && splitTree(splitRoot, point, {
        isSkipPaddingBlankHTML: isInline,
        isNotSplitEdgePoint: isInline
    });
    // if container is point.node, find pivot with point.offset
    if (!pivot && container === point.node) {
        pivot = point.node.childNodes[point.offset];
    }
    return {
        rightNode: pivot,
        container: container
    };
}

the code finds the ancestor of the selection / insert point, which is typically a p element, and duplicates this ancestor element, then inserts the pasted content into the new element. If your markup being pasted contains block elements, these are each wrapped with the ancestor due to the loop through the child nodes in pasteHtml , which results in additional ancestor elements inserted around every child node. referenced same issue here https://github.com/summernote/summernote/issues/1942#issuecomment-393253948

stevenyoon commented 5 years ago

hello! this seems to still be an issue in v0.8.11-- i am using the pasteHtml function with the html:

{% if CONDITION %}<br />                    {% elif CONDITION %}<br />                    {% else %}<br />                    {% endif %}

it doesn't occur when pasting it into an empty summernote, but does if there is already text there.

Please let me know if i can provide more information!

Edit: for anyone else running into this issue, we were able to solve this with something like:

// focus on the dom element because execCommand only works on a selected element
$(this.props.summernoteElt).next().find(".note-editable").focus()

// call document.execCommand here because summernote has a bug that reverses
// the text being pasted in for whatever reason
document.execCommand("insertHtml", false, htmlValue)
foaadnami commented 5 years ago

@lqez i can confirm that this is an issue in 0.8.11 too

erickbandinelli commented 5 years ago

Hello everyone, use this and you will no longer have problems with the editor, nor the duplication of the text nor the removal of the formatting.

$('[data-summernote-editor]').each(function () {

    var $self = $(this);
    $self.summernote({
        height: $self.attr('data-height') || 400,
        // blockquoteBreakingLevel: 0,
        toolbar: [
            ['cleaner', ['cleaner']],
            ['style', ['bold', 'italic', 'underline', 'clear']],
            ['para', ['style', 'ul', 'ol', 'paragraph', 'table']],
            ['insert', ['picture', 'link', 'video']],
            ['misc', ['fullscreen', 'codeview']]
        ],
        callbacks: {
            onPaste: function (e) {
                var bufferText = ((e.originalEvent || e).clipboardData || window.clipboardData).getData('Text');
                e.preventDefault();
                // Firefox fix
                setTimeout(function () {
                    document.execCommand('removeFormat', false, bufferText);
                }, 10);
            }
        },
        cleaner: {
            action: 'both', // both|button|paste 'button' only cleans via toolbar button, 'paste' only clean when pasting content, both does both options.
            newline: '<br>', // Summernote's default is to use '<p><br></p>'
            notStyle: 'position:absolute;top:0;left:0;right:0', // Position of Notification
            icon: '<i class="note-icon">Clear Style</i>',
            keepHtml: false, // Remove all Html formats
            keepOnlyTags: ['<p>', '<br>', '<ul>', '<ol>', '<li>', '<b>', '<strong>', '<i>', '<a>'], // If keepHtml is true, remove all tags except these
            keepClasses: false, // Remove Classes
            badTags: ['style', 'script', 'applet', 'embed', 'noframes', 'noscript', 'html'], // Remove full tags with contents
            badAttributes: ['style', 'start'], // Remove attributes from remaining tags
            limitChars: false, // 0/false|# 0/false disables option
            limitDisplay: 'both', // text|html|both
            limitStop: false // true/false
        }
    });
});
junkindm commented 5 years ago

In the summernote-lite.js file (version 0.8.11), I updated the first if statement to rng.so >= 0 instead of rng.so > 0. It seems to have worked for me.

  WrappedRange.prototype.pasteHTML = function (markup) {
      var contentsContainer = $$1('<div></div>').html(markup)[0];
      var childNodes = lists.from(contentsContainer.childNodes);

      var rng = this.wrapBodyInlineWithPara().deleteContents();
      if (rng.so >= 0) {
          childNodes = childNodes.reverse();
      }
      childNodes = childNodes.map(function (childNode) {
          return rng.insertNode(childNode);
      });
      if (rng.so > 0) {
          childNodes = childNodes.reverse();
      }
      return childNodes;
  };
diegojlucena commented 5 years ago

Worked for me too.

animha commented 5 years ago

Have the same issue with version 0.8.11. I used junkindm fix and seems to work fine for me.

yuriy-harbuzyuk commented 5 years ago

Same issue with 0.8.12 in Chrome. Following API inserts html in reverse order when current position is start of line $('#summernote')).summernote('editor.pasteHTML','line1
\nline2')

foaadnami commented 5 years ago

Anyone has an idea as to why this three year old bug which is labelled as "Bug" and "Priority High" hasn't been addressed in any of the last 10 releases since this first got reported ? Just curious :)

mrov commented 4 years ago

Ohhhhh god, its still open 😢

DennisSuitters commented 4 years ago

IKR, I have been waiting for the PR for this to be fixed, as @pascalbaljet edited the files in the dist/ folder I am waiting for them to amend those changes so I can check the PR before hopefully merging it.

DennisSuitters commented 4 years ago

This has now been merged after testing. If anyone finds issues still, please open a new issue filling out the issue template to help us work out issues in a timely manner. Thanks to everyone for being so patient.