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

Rich text copy/cut/paste #237

Open
wants to merge 29 commits into
base: master
Choose a base branch
from
Open

Conversation

kofifus
Copy link
Contributor

@kofifus kofifus commented Feb 16, 2016

Demo:
plunker

Notes:

  • Does not work when Chorme developer tools is open (not sure why), will revert to CM copy
  • requires a recent codemirror > 5.13.2
  • Make sure you have lineWiseCopyCut:false in your CM options

@mikelehen
Copy link
Collaborator

Thanks for this! I'll take a look and review it when I get a chance. Unfortunately I'm pretty busy with other projects at the moment, so there may be a bit of a delay.

@kofifus
Copy link
Contributor Author

kofifus commented Mar 23, 2016

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

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

Obviously you need CodeMirror 5.12

@kofifus
Copy link
Contributor Author

kofifus commented Mar 28, 2016

Please hold on with merging this. There may be a better way, see https://discuss.codemirror.net/t/clipboarddata-setdata/695

@kofifus
Copy link
Contributor Author

kofifus commented Apr 5, 2016

I have updated the PR again with much better results for mac OSX and better style handling on paste. I think I will leave this for now unless any issues are found. I hope this can be merged soon.

Updated http://plnkr.co/edit/PgMKReE64a8Nc4HiTx4I?p=preview

Rember this PR requires Codemirror > 5.13
you also want cm option lineWiseCopyCut:false

@jbrecht
Copy link

jbrecht commented Apr 15, 2016

@kofifus - we have a fork of firepad for some customizations we are making to it for our learning/therapy application. this copy/paste functionality is something we'd love to have. if you are open to it, I'd love to pull this branch into our fork and I can do some experimentation and further code review. https://github.com/presencelearning/firepad

@kofifus
Copy link
Contributor Author

kofifus commented Apr 15, 2016

yeah sure do I need to do anything ?

@@ -90,7 +90,6 @@ firepad.Firepad = (function(global) {
}
this.client_ = new EditorClient(this.firebaseAdapter_, this.editorAdapter_);

var self = this;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deleting this definition of 'self' breaks a lot of things later on

@jbrecht
Copy link

jbrecht commented Apr 18, 2016

@kofifus - this is not working for me. After I fixed the missing 'self' definition, I now run into an issue where onCodeMirrorPaste_ quits early because it gets no 'html'. The clipboardData in the event it receives has an empty 'types' array. So then I went to look at what happens in onCodeMirrorCopyCut_. If I log the final e.clipboardData, it monetarily has a 'types' array of length 1, but it becomes length 0 shortly after the call exits, so something is wiping it out. (Using Chrome 50 on OS X.)

@kofifus
Copy link
Contributor Author

kofifus commented Apr 18, 2016

apologies the removed 'self' crept in from another PR I'm working on, fixed.

The other problem you specify may be a mac OS problem, can you check with http://plnkr.co/edit/PgMKReE64a8Nc4HiTx4I?p=preview and see how it goes there and let me know pls ?

@jbrecht
Copy link

jbrecht commented Apr 18, 2016

The Plunker demo works in Firefox (45) but fails in Chrome (50). I'm on OS X 10.10.5.

@kofifus
Copy link
Contributor Author

kofifus commented Apr 18, 2016

ok, this may be related to https://bugs.chromium.org/p/chromium/issues/detail?id=552975

I should have access to a mac in the next few days and will have a look thx

@kofifus
Copy link
Contributor Author

kofifus commented Apr 23, 2016

jbrecht, I am checking on MacBook Pro / OS X Yosemite 10.10.1 / Chrome version 50.0.2661.86 (64-bit) using the plunkr.

Everything seems to work ok, that is copy/paste of text with rich text attributes (bold etc) works properly.

Can you test again ? what exact computer / OSX version / Chrome version you're using ?

@kofifus
Copy link
Contributor Author

kofifus commented Apr 24, 2016

further commits to fix a problem in parseHtml that resulted in buggy behavior when pasting a line with leading spaces

new plunkr: http://plnkr.co/edit/MqAdtT1eNCdyheWaKnGO?p=preview

@jbrecht
Copy link

jbrecht commented May 16, 2016

This works for me, although a unit test is currently failing

@kofifus
Copy link
Contributor Author

kofifus commented May 17, 2016

Thx John, the unit auto build system has issues with npm package versioning etc .. if you feel like helping that will be great:)

PS there still seems to be issues with rich text copy/paste in mac Chrome which I'm looking into

@kofifus
Copy link
Contributor Author

kofifus commented May 19, 2016

Updated the code to correctly remove sentinels when doing plain text copy/cut
(http://plnkr.co/edit/MqAdtT1eNCdyheWaKnGO)

No further known issues at this stage :)

@llamerr
Copy link

llamerr commented Jun 15, 2016

I also interested in this pull request, I squashed it and merged into my version (1 year old, needed to be updated), after solving few minor conflicts it's working fine. But it's not working in Edge due to lack of 'text/html' support for Clipboard API. When I replaced it with 'text' in setData/getData it started working in Edge and continued working in Chrome. I haven't tested it extensively yet so not sure if that can be kept, but maybe it can detect if after using setData clipboard contains something, and if not, then re-try with 'text' instead of 'text/html'?

@kofifus
Copy link
Contributor Author

kofifus commented Jun 15, 2016

thx ... interesting

in https://html.spec.whatwg.org/multipage/interaction.html#dom-datatransfer-getdata I find:

'If format equals "text", change it to "text/plain".'
that means that if edge complies with the standard above, if you change "text/html" to "text" you will get "text/plain" that is you will have plain text copy/paste and not rich text copy/paste.

Do you find that you still get rich text copy/paste (ie bold/italics/etc) with that change ? I find that strange ..

@llamerr
Copy link

llamerr commented Jun 15, 2016

Yes, I think it's working because you only using Clipboard API for storing text, not actually for copy/pasting it (I'm new to it and haven't read docs yet, so not sure if there other ways to working with it and what clipboard contains when you copy html)
val=fp.getHtmlFromSelection(); for copy
and
fp.insertHtmlAtCursor(html); for paste
is used, so that value is string and can be stored as string inside clipboard.

@kofifus
Copy link
Contributor Author

kofifus commented Jun 15, 2016

ha you're right, this will work within firepad, but it will not paste rich text outside (for example into a gmail msg), or the other way around .. can you check ?

@llamerr
Copy link

llamerr commented Jun 15, 2016

Yes, you're right, that's the difference! It's pasting raw html ('<span ...') in case of 'text' and decorated text in case of 'text/html', so seems not the best option to use that by default.

@kofifus
Copy link
Contributor Author

kofifus commented Jun 15, 2016

another win for microsoft ...

still you can test for edge and use 'text' in that case which will give you rich copy text inside firepad which is much better than nothing

llamerr pushed a commit to invigos/firepad that referenced this pull request Jun 15, 2016
…nd added changes for copy/cut/paste buttons using same functionality
@kofifus
Copy link
Contributor Author

kofifus commented Jun 23, 2016

Michael, I'm about to update this with a few more changes and want to use the new FireBase version.
Is there a way for you to provide a public FB instance (config = { apiKey: .., authDomain: ..., databaseURL: ..., storageBucket: ... };) that I can use for the plunkr demo ?

As this PR seems to be important and needs a lot of testing this will be very helpful.

Thx!

@mikelehen
Copy link
Collaborator

Since we don't need authentication / security rules, you should actually be able to just use https://firepad.firebaseio-demo.com as the databaseURL and set the other fields to dummy values (empty string or whatever).

@kofifus
Copy link
Contributor Author

kofifus commented Jun 23, 2016

I committed further updates to the code -

I removed the surrounding span around the copied clip that had 'default' styles like font family/size/etc. This span caused issues when pasting into rich text editors (ie openoffice) and is not essential. FireFox for example does not add such default attributes into a copy clip (chrome does). This made the code much cleaner and works better when pasting outside FP.

The original FP code serializeHTML replaced all leading trailing and consecutive spaces with nbsp .. this had a negative effect of creating hard spaces when pasting into for example open office. I now removed that code and instead serializeHTML adds <pre> .. </pre> around the copy div

Also the copy clip will now be saved in the clipboard in both html and text formats. This allows copying rich text from FP and pasting into non rich text editors or rich text editors that cannot read html clips from the clipboard (ie wordpad).

Plunkr: plunkr

@mikelehen
Copy link
Collaborator

Awesome. Those sound like good improvements. :-)

On Thu, Jun 23, 2016 at 4:45 PM, Kofifus [email protected] wrote:

I committed further updates to the code -

I removed the surrounding span around the copied clip that had 'default'
styles like font family/size/etc. This span caused issues when pasting into
rich text editors (ie openoffice) and is not essential. FireFox for example
does not add such default attributes into a copy clip (chrome does). This
made the code much cleaner and works better when pasting outside FP.

Also the copy clip will now be saved in the clipboard in both html and
text formats. This allows copying rich text from FP and pasting into non
rich text editors or rich text editors that cannot read html clips from the
clipboard (ie wordpad).

Plunkr: plunkr http://plnkr.co/edit/dcu96xxQYA7yB0htNqu6?p=preview


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#237 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAMmHPDOpseEO2YZrPDiYJ5aQlX8YOfdks5qOxqcgaJpZM4HaxCz
.

@Haraldlons
Copy link

Thanks for the work @kofifus!

I'm not able to get any of the plunkr to work, copying from the editor and pasting into gmail/slack.

Using Google Chrome (Version 90.0.4430.212 (Official Build) (64-bit) (Ubuntu/Linux))

Is there any update on this PR?

This feature would be highly valuable to my team!

@kofifus
Copy link
Contributor Author

kofifus commented Jun 23, 2021

well it's been 5 years and TBH I totally gave up in firepad as it's not really supported .. you can find projects binding the amazing ProseMirror to FireBase .. sorry can't help

@samtstern
Copy link
Contributor

samtstern commented Jun 23, 2021

@kofifus I am sorry this PR was not given the attention it deserved when you originally filed it. If anyone wants to try and revive this PR I would be happy to review it.

We've tried to clarify the state of this project in the README since then: https://github.com/FirebaseExtended/firepad#status

Basically we (Firebase) are not actively working on this project however we're happy to accept Pull Requests and release new updates when required. There are many people who depend on Firepad and we believe it to be a mostly healthy library as-is.

@Haraldlons

This comment has been minimized.

@samtstern
Copy link
Contributor

Ah whoops! I meant this link: https://github.com/FirebaseExtended/firepad#status

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

Successfully merging this pull request may close these issues.

None yet

6 participants