Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Copy/Paste should preserve styling #35

Open
iclems opened this issue Jul 12, 2013 · 42 comments
Open

Copy/Paste should preserve styling #35

iclems opened this issue Jul 12, 2013 · 42 comments

Comments

@iclems
Copy link
Contributor

iclems commented Jul 12, 2013

There are three discussions here:

  • how to make rich copy/paste fully compatible within Firepad
  • how to preserve rich copy/paste fully compatible within Firepad from interfering with other apps (e.g. the issue with special chars appearing when you copy paste in other apps => oncopy event ?)
  • how to make rich copy/paste compatible with other apps (mainly I guess to make basic styling compatible with apps as a Text Editor / Word)
@iclems
Copy link
Contributor Author

iclems commented Nov 14, 2013

The easiest way of doing this seems to me to play with event.clipboardData.getData('text/firepad') and setData('text/firepad') (or using directly the mime/type 'text/html' which is slightly more risky as Chrome will provide rich html content for data pasted from others apps). Then, using HTML as a bridge for import/export (as we currently do), would require for these operations to have getHtmlFromSelection (or getHtmlFromRange ?) and insertHtmlAtIndex/Cursor. How does that seem to you?

@mikelehen
Copy link
Collaborator

That sounds great, but I don't think clipboardData is widely supported (it might be IE-only?). In general, I believe browsers make it difficult to access the clipboard directly, due to security concerns. So making copy/paste work well will likely require some browser hackery. The typical approach (I think) is to detect a ctrl-C operation and move the focus / selection to some place that has the content that you want to end up in the clipboard. Similarly for ctrl-V, you paste to some alternate location and then process the pasted content and insert where you want it. This hackery is why I haven't felt like tackling this. :-)

@iclems
Copy link
Contributor Author

iclems commented Nov 14, 2013

I did think the same. But the fact is that window.clipboardData is supported by IE, and at least Chrome+Safari support event.clipboardData getData and setData (I've tested this). So it made me change a bit my mind ;-)

  • Plain text: I've played with CodeMirror input and it's pretty easy to fix the sentinel chars being copied (just need to catch the 'copy' event and replace the inputField value which has already been updated with current selection value)
  • HTML: CodeMirror uses a textarea inputfield to handle copy / paste (and input, as you know). The issue is that text area are considered plain text only by all browsers, except Chrome. Hence, only Chrome receives an HTML representation of the data pasted in a text area. This basically means that paste from external apps could preserve attributes in Chrome, but I understand it's not a real solution... Yet, changing CodeMirror input for a div with contenteditable=true does not seem a possible way either.
  • Short term: getHtmlFromSelection and insertHtmlAtCursor fix the issue of copy/paste within Firepad. I've just created a inserHtmlAtCursor based on setHtml: that fixes the paste part of the thing as we can capture input.on('paste', function(event) { var html = event.clipboardData.getData('text/firepad'); firepad.insertHtmlAtCursor(html); }).

Do you have any suggestion for a getHtmlFromSelection?

@mikelehen
Copy link
Collaborator

Perhaps this will help (add it in firepad.js somewhere):

Firepad.prototype.getOperationForSpan = function(start, end) {
var text = this.richTextCodeMirror_.getRange(start, end);
var spans = this.richTextCodeMirror_.getAttributeSpans(start, end);
var pos = 0;
var op = new firepad.TextOperation();
for(var i = 0; i < spans.length; i++) {
op.insert(text.substr(pos, spans[i].length), spans[i].attributes);
pos += spans[i].length;
}
return op;
};

You can use that to create a TextOperation representing the span, and then
the existing getHtml() logic should work on that.

If you can get something working, I'd be thrilled with a pull request. :-)

On Thu, Nov 14, 2013 at 11:50 AM, Clément Wehrung
[email protected]:

I did think the same. But the fact is that window.clipboardData is
supported by IE, and at least Chrome+Safari support event.clipboardData
getData and setData (I've tested this). So it made me change a bit my mind
;-)

  • Plain text: I've played with CodeMirror input and it's pretty easy
    to fix the sentinel chars being copied (just need to catch the 'copy' event
    and replace the inputField value which has already been updated with
    current selection value)
  • HTML: CodeMirror uses a textarea inputfield to handle copy / paste
    (and input, as you know). The issue is that text area are considered plain
    text only by all browsers, except Chrome. Hence, only Chrome receives an
    HTML representation of the data pasted in a text area. This basically means
    that paste from external apps could preserve attributes in Chrome, but I
    understand it's not a real solution... Yet, changing CodeMirror input for a
    div with contenteditable=true does not seem a possible way either.
  • Short term: getHtmlFromSelection and insertHtmlAtCursor fix the
    issue of copy/paste within Firepad. I've just created a inserHtmlAtCursor
    based on setHtml: that fixes the paste part of the thing as we can
    capture input.on('paste', function(event) { var html =
    event.clipboardData.getData('text/firepad');
    firepad.insertHtmlAtCursor(html); }).

Do you have any suggestion for a getHtmlFromSelection?


Reply to this email directly or view it on GitHubhttps://github.com//issues/35#issuecomment-28516445
.

@brianburt
Copy link

I'm very curious if anyone has worked on this... Google Docs handles it so well, I think now the user expectation is that this work. We're using firepad inside an app, and so are very interested.

@mikelehen
Copy link
Collaborator

I think @iclems made progress on this at one point. Hopefully he'll chime in. :-)

@brianburt
Copy link

brianburt commented Oct 9, 2014

Thanks Michael.
Clement any news on html copy paste in firepad?
#35

On Wed, Oct 8, 2014 at 6:24 PM, Michael Lehenbauer <[email protected]

wrote:

I think @iclems https://github.com/iclems made progress on this at one
point. Hopefully he'll chime in. :-)

Reply to this email directly or view it on GitHub
#35 (comment).

Brian Burt
Chief Visioneer (Founder & CEO)

Leader in Social Conferencing
www.MaestroConference.com http://www.maestroconference.com/

We are the leading technology for hosting real conversations at large
scale. Customers include the Obama Campaign who used us for 35,000
events, the National Association of Realtors, and the National Science
Foundation
; other users include authors such as Jack Canfield, Marianne
Williamson, Deepak Chopra among thousands of others worldwide using
MaestroConference.

@iclems
Copy link
Contributor Author

iclems commented Oct 10, 2014

You’ll find here a quick example of how to do it: https://gist.github.com/iclems/31b44bb7aba9bf7713a8

Note that if you use entities, you probably need some additional processing, etc. 

Clément

On Fri, Oct 10, 2014 at 12:38 AM, brianburt [email protected]
wrote:

Thanks Michael.
Clement any news on html copy paste in firepad?
#35
On Wed, Oct 8, 2014 at 6:24 PM, Michael Lehenbauer <[email protected]

wrote:
I think @iclems https://github.com/iclems made progress on this at one
point. Hopefully he'll chime in. :-)

Reply to this email directly or view it on GitHub
#35 (comment).

Brian Burt
Chief Visioneer (Founder & CEO)
+1 415 727 1820 direct
+1 877 414 1515 customer support
Leader in Social Conferencing
www.MaestroConference.com http://www.maestroconference.com/
We are the leading technology for hosting real conversations at large
scale. Customers include the Obama Campaign who used us for 35,000
events, the National Association of Realtors, and the National Science
Foundation
; other users include authors such as Jack Canfield, Marianne
Williamson, Deepak Chopra among thousands of others worldwide using

MaestroConference.

Reply to this email directly or view it on GitHub:
#35 (comment)

@nickgrossman
Copy link

Thanks -- I just looked at that Gist and couldn't quite figure how to integrate it. I was able to update it with correct references to my firepad and codeMirror instances, but choked on the "LineSentinelCharacter+EntitySentinelCharacter" variables.

Any suggestions?

Thanks -- this would be a really awesome feature to have

@oresmus
Copy link

oresmus commented Sep 1, 2015

@nickgrossman (and/or @brianburt), I'd be happy to try to help with this (and eventually submit a pull request), since I'd like the same feature in Firepad. But I don't quite see how that gist is supposed to fit in, so if you or @iclems have any public code sample that uses Firepad and incorporates that gist (even if it's disabled since it's not yet fully correct), that would help me get started. (FYI, the "sentinel" variables are internal variables of some object in firepad.js; if they were just commented out in the gist, I think it would still "sort of work" and be a good version to start with. They also happen to have constant values.) [update: now I think I see how to incorporate the gist, so any code sample is not urgent.]

@oresmus
Copy link

oresmus commented Sep 1, 2015

As I mentioned in #199, it looks to me like the gist above by @iclems would fix #199 as well as adding this feature.

@iclems
Copy link
Contributor Author

iclems commented Sep 1, 2015

The gist does work fine. Though, you can actually improve it by setting a private mime type for Firepad to distinguish copy/paste within a firepad or from outside. When a paste comes from outside, you may need to perform additional cleanup before trying to insert the pasted HTML or text.

Also, some entities may require to be able to perform some specific tasks when being inserted through a paste. Think of a paste of rich text which includes images: in this scenario, the clipboard should contain an HTML version of the rich text (at least on Mac), though you'll probably need to upload the base64 or image URL to your server to store it, and get a token to store in Firepad, etc.

Hence, it would take some work to figure a really generic implementation of copy/paste for rich text that would work in every case and have full support for entities.

@oresmus
Copy link

oresmus commented Sep 1, 2015

@iclems, thanks for this additional info. It sounds much easier to just preserve basic styling, and that is what's high-priority for my use case, so I'm planning to try to get that to work (and fix #199), and after that we'll see (maybe splitting off a separate issue for going farther will make sense).

One question about your gist: does the ios case (shown below) work correctly, or does it always copy the entire input element? (I am not familiar with web programming, so I am just guessing the details of the DOM interface used below when assigning to node.selectionEnd, etc.)

function selectInput(node) {
  if (ios) { // Mobile Safari apparently has a bug where select() is broken.
    node.selectionStart = 0;
    node.selectionEnd = node.value.length;
  } else {
    // Suppress mysterious IE10 errors
    try { node.select(); }
    catch(_e) {}
  }
}

jfrazzano added a commit to johnegotta/FC_Polymer_Shared that referenced this issue Nov 11, 2015
…the August posted work on github to use a rtf cut and paste that actually maintains formatting.

here is the link on git hub to the working code that is currently being imported for a pull, hopefully....

source: with other good links

FirebaseExtended/firepad#35

specific link (last year)

https://gist.github.com/iclems/31b44bb7aba9bf7713a8

recent comments: FirebaseExtended/firepad#199


I am going to do my best to impliment a direct feed of text to our word array creator, and keep formatting... huge plus
@kofifus
Copy link
Contributor

kofifus commented Dec 14, 2015

Hi,

iclems gist above does not work anymore for several reasons

  • clipboardData.setData is deprecated and already does not work on some browsers. Working with the clipboard is now more involved and uses the document.execCommand commands
  • doing e.preventDefault from addEventListener('paste', ) is broken in CM and the text will be inserted twice
  • FP inserts an extra newline at the end of html inserts which prevents proper html mid line insertions
  • Styling issues with Fonts and bg color

The code below shows a solution to these problems:

See code example in
http://jsfiddle.net/radcct7e/

rich text can be copied/cut/pasted inside FP and from/to a richpad document (ie gmail msg)

consider adding lineWiseCopyCut:false to CM options to prevent cut without selection

This was tested in IE9-11, FF and Chrome

Note that for some reason this does not work when chrome developer tools is opened.

I am working towards a PR for this. Any feedback appreciated

Code:

var firepadRef = new Firebase('https://firepad.firebaseio-demo.com').child('say9rRbxEY');
var codeMirror = CodeMirror(document.getElementById('firepad-container'), { lineWrapping: true, lineWiseCopyCut:false });
var firepad = Firepad.fromCodeMirror(firepadRef, codeMirror, { richTextToolbar: true, richTextShortcuts: true });

codeMirror.getWrapperElement().addEventListener('copy', function(e) { onFirepadCopy(firepad, e); });
codeMirror.getWrapperElement().addEventListener('cut', function(e) { onFirepadCut(firepad, e); });
codeMirror.on('paste', function(cm, e) { onFirepadPaste(firepad, e); });

function onFirepadCopy(fp, e) {
  if (!e.clipboardData) return; // older browsers
  var cm=fp.codeMirror_;
  var html=fp.getHtmlFromSelection();
  var text=cm.getSelection();
  if (!html || html=='<div>'+text+'</div>') return; // not html

  // caching some default styles needed later
  if (!cm.defaultStyles) {
    var style = window.getComputedStyle(cm.getWrapperElement());
    cm.defaultStyles={
      fontFamily: style.getPropertyValue('font-family'),
      fontSize: style.getPropertyValue('font-size'),
      bgColor: style.getPropertyValue('background-color'),
    };
  }

  // add default font to the html
  html='<span style="font-family:'+cm.defaultStyles.fontFamily+';font-size:'+cm.defaultStyles.fontSize+'">'+html+'</span>';

  if (!copyHtmlToClipboard(html)) return;
  console.log('copied '+html);
  e.preventDefault();
}

function onFirepadCut(fp, e) {
  if (!e.clipboardData) return; // older browsers
  var cm=fp.codeMirror_;
  onFirepadCopy(fp, e);
  cm.replaceSelection('');
}

function onFirepadPaste(fp, e) {
  if (!e.clipboardData) return; // older browsers
  var cm=fp.codeMirror_;

  var html = e.clipboardData.getData('text/html');
  if (!html) return null; // non rich text paste

  // setting bg color when not needed can break selection, so removing it here
  if (cm.defaultStyles) html = html.split('background-color: '+cm.defaultStyles.bgColor).join('background-color:transparent');

  fp.insertHtmlAtCursor(html);
  console.log('pasted '+html);
  e.preventDefault();
}

function copyHtmlToClipboard(html, extraCSS) {
  var div = document.createElement('div');
  if (extraCSS) div.style.cssText = extraCSS;
  div.style.opacity = 0;
  div.style.position = 'absolute';
  div.style.pointerEvents = 'none';
  div.style.zIndex = -1;
  div.setAttribute('tabindex', '-1'); // so it can be focused
  div.innerHTML = html;
  document.body.appendChild(div);

  var focused=document.activeElement;
  div.focus();

  window.getSelection().removeAllRanges();
  var range = document.createRange();
  // not using range.selectNode(div) as that makes chrome add an extra <br>
  range.setStartBefore(div.firstChild);
  range.setEndAfter(div.lastChild);
  window.getSelection().addRange(range);

  var ok=false;
  try {
    ok = document.execCommand('copy');
  } catch (err) {
    console.log(err);
  }
  if (!ok) {
    console.log('execCommand failed!');
    return false;
  }

  window.getSelection().removeAllRanges();
  document.body.removeChild(div);

  focused.focus();
  return true;
}

@danielschwartz
Copy link

@kofifus and Firepad team. I've implemented this and while it works, the editor freezes on insert of more than a couple lines. After doing some digging in the Chrome Profiler it seems that its the internals of Firepad causing the extreme slowness. Here is a screenshot of my profiler:

profiler

Happy to send you guys my CPU profile from Chrome. Is this a known issue or am I doing something wrong here?

@mikelehen
Copy link
Collaborator

Hrm. I don't see anything obvious from that screenshot. Looks like the bulk of the expanded time actually ends up in markText(), which is a CodeMirror API... but maybe firepad is doing something inefficient in how it's using the API.

I'm not sure when I'll have time to dig into this, but any chance you could provide a pull request or jsfiddle or something that has your changes so I can debug / profile myself? That'd probably be most useful.

@kofifus
Copy link
Contributor

kofifus commented Feb 8, 2016

Hi Daniel,

Thank you for the feedback !

  • I could not reproduce the editor freezing but can see a delays of a few secs on some occasions is that what you meant or did you see a total freeze ?
  • Did you highlight the particular profiler line on purpose ?
  • I could also not reproduce delays on normal text copy so it seems the problem is in the rich text code above or it's interaction with FP

I will investigate further

@danielschwartz
Copy link

@mikelehen will try to get any changes i've made on Firepad internal into a pull request or something similar. FWIW -- I also see a fairly slow init on long documents with a bunch of rich text, it might have something to do with how much of the document is being rendered. Ill dig deeper and see if I can figure it out.

Edit: The slow initial load is based on how much of the actual document your rendering in CodeMirror. Basically, don't use viewportMargin: Infinity in CodeMirror, set a specific height.

My original issue with copy paste (which uses insertHtml) still stands though.

@danielschwartz
Copy link

@kofifus realized I never answered your questions:

  • I see both, but they are likely caused by the same underlying issue. If you paste in a long document, it will freeze, but not come out of it ever because the thing you are pasting is large. I've been able to reliably get tab crashes on Chrome using paste.
  • Nope, sorry about that.
  • Agreed

My next place to look will be in the places where Firepad subscribes to CodeMirror events. In my fork of Firepad i've added onPasteStart and onPasteEnd events so that changes and cursorActivity isn't run every time RichTextCodeMirror calls insertText for the textPieces.

Also interested in seeing if its possible to batch AnnotationSpan inserts/updates, but not sure that CodeMirror allows you to somehow batch markText calls. If anyone has any good ideas about that, let me know!

@kofifus
Copy link
Contributor

kofifus commented Feb 9, 2016

I opened #229

@cben
Copy link
Contributor

cben commented Feb 9, 2016

You can use cm.operation around markText. I've done it in a different use
case but it helped very little.
9 февр. 2016 г. 0:44 пользователь "Daniel Schwartz" <
[email protected]> написал:

@kofifus https://github.com/kofifus realized I never answered your
questions:

  • I see both, but they are likely caused by the same underlying issue.
    If you paste in a long document, it will freeze, but not come out of it
    ever because the thing you are pasting is large. I've been able to reliably
    get tab crashes on Chrome using paste.
  • Nope, sorry about that.
  • Agreed

My next place to look will be in the places where Firepad subscribes to
CodeMirror events. In my fork of Firepad i've added onPasteStart and
onPasteEnd events so that changes and cursorActivity isn't run every time
RichTextCodeMirror calls insertText for the textPieces.

Also interested in seeing if its possible to batch AnnotationSpan
inserts/updates, but not sure that CodeMirror allows you to somehow batch
markText calls. If anyone has any good ideas about that, let me know!


Reply to this email directly or view it on GitHub
#35 (comment).

@mikelehen
Copy link
Collaborator

Hrm. Definitely interesting that the setHtml is taking longer than initial load. That probably suggests there's some improvement that could be made like wrapping it in a CodeMirror operation as Beni suggested.

@danielschwartz
Copy link

@mikelehen ill probably have some time today or tomorrow to test this theory out. Will report back and see if theres any substantial gain from doing this.

@danielschwartz
Copy link

@mikelehen, @kofifus reporting back. Wrapping insertHtmlAtCursor with a CodeMirror operation improves insertion immensely. Copy pasting the same document, the one with the operation took about 8 seconds (it's a fairly long and rich text heavy document) and without the operation my tab actually hung and then crashed. So it seems wrapping it in an operation not only makes the changes happen faster but is more stable. I don't know when ill have time to submit a pull request with a vetted copy/paste solution, but just wanted to post here in case anyone else ran into this issue.

Update: finally got my browser not to crash. Here are the numbers:

  • Without CodeMirror Operation: 110124.5 ms
  • With CodeMirror Operation: 8052.9 ms

So, yeah. Much better wrapped in an operation.

@mikelehen
Copy link
Collaborator

Awesome, thanks for reporting back! Would love to see that PR if/when you
have time. :-)

On Tue, Feb 9, 2016 at 10:06 AM, Daniel Schwartz [email protected]
wrote:

@mikelehen https://github.com/mikelehen, @kofifus
https://github.com/kofifus reporting back. Wrapping insertHtmlAtCursor
with a CodeMirror operation improves insertion immensely. oCopy pasting the
same document, the one with the operation took about 8 seconds (it's a
fairly long and rich text heavy document) and without the operation my tab
actually hung and then crashed. So it seems wrapping it in an operation not
only makes the changes happen faster but is more stable. I don't know when
ill have time to submit a pull request with a vetted copy/paste solution,
but just wanted to post here in case anyone else ran into this issue.


Reply to this email directly or view it on GitHub
#35 (comment).

@kofifus
Copy link
Contributor

kofifus commented Feb 9, 2016

great daniel ! can you post an updated version of the fiddle from my solution above ? (http://jsfiddle.net/radcct7e/)

@danielschwartz
Copy link

Updated: http://jsfiddle.net/radcct7e/3/

@kofifus
Copy link
Contributor

kofifus commented Feb 12, 2016

daniel, speed issue is now "solved" with d0459a8

updated firepad.js with this fix is in https://github.com/firebase/firepad/blob/master/examples/firepad.js

@danielschwartz
Copy link

Awesome! Super fast turn around on both the PR and acceptance. Thanks!

Daniel Schwartz

On Thu, Feb 11, 2016 at 8:42 PM Kofifus

<
mailto:Kofifus [email protected]

wrote:

daniel, speed issue is now solved with
d0459a8

Reply to this email directly or
#35 (comment)
.

#35 (comment)

@kofifus
Copy link
Contributor

kofifus commented Feb 16, 2016

#237

@nickgrossman
Copy link

Hi All--

This is exciting! (for some reason the other notifications on this thread
got buried so I'm just seeing them now).

I will take a look at this. Thanks everyone for the effort!

Best,
Nick

On Monday, February 15, 2016, Kofifus [email protected] wrote:

#237 #237


Reply to this email directly or view it on GitHub
#35 (comment).

http://nickgrossman.is | @nickgrossman http://twitter.com/nickgrossman

@domalaq
Copy link

domalaq commented Mar 8, 2016

Hello everyone!
Copy works great in the fiddle from @danielschwartz above. But Cut doesn't :(

@kofifus
Copy link
Contributor

kofifus commented Mar 8, 2016

Hey domalak, daniel's fiddle is based on mine above which has been improved a lot and submitted as a PR - pls see #237

@domalaq
Copy link

domalaq commented Mar 9, 2016

oh thanks a lot @kofifus

@moinahmed
Copy link

Are we going to mark this issue as closed?

@mikelehen
Copy link
Collaborator

Probably not until #237 is merged and I'm swamped at the moment. Sorry!

@kofifus
Copy link
Contributor

kofifus commented Mar 9, 2016

clipboard manipulation in browsers is difficult and not well documented/supported (for example as far as I could tell not feasible in IE), any help with testing will be greatly appreciated. The PR was only tested in Chrome and FF but there may be issues with safari/phones/tablets or with some formatting combinations.

@kofifus
Copy link
Contributor

kofifus commented Mar 23, 2016

CodeMirror 5.12 now supports copy & cut events which makes the firepad code simpler. I have updated the #237 accordingly and also enhanced some styles support.

New plnkr in http://plnkr.co/edit/fMroiV79FvkCJF590dpj

Obviously you need CodeMirror 5.12 ..

Hope this helps

@moinahmed
Copy link

When can we see this in main repo?

@mikelehen
Copy link
Collaborator

Sorry, I'm not sure. I'm a little nervous about the change and haven't had time to do a detailed review / testing. If people can use #237 for now and chime in to let me know how it's working and whether they've hit any issues, that would help some. :-)

@kofifus
Copy link
Contributor

kofifus commented Jun 18, 2016

yeah #237 is definitely a complicated PR .. I would really appreciate more people testing ... it seems to work great on chrome/firefox/safari desktops, didn't really test much on mobiles ... I tried to make it so it reverts to default behavior in places where the clipboard api is not supported

@moinahmed
Copy link

I'm going to use #237 in my test environment; will update here if come across any issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants