Introduce paste from libre office plugin #3624

msamsel · 2019-11-04T13:36:00Z

What is the purpose of this pull request?

New feature

Does your PR contain necessary tests?

All patches which change the editor code must include tests. You can always read more
on PR testing,
how to set the testing environment and
how to create tests
in the official CKEditor documentation.

This PR contains

Unit tests
Manual tests

What is the proposed changelog entry for this pull request?

Added support for pasting rich content from Libre Office with the [Paste from Libre Office](https://ckeditor.com/cke4/addon/pastefromlibreoffice) plugin.

What changes did you make?

Requires: #3593

Implement part of the content fixing mechanism inside the common filter. Thi result with fixing some of the wrong expected output for IE11 & Safari on PFW.
The plugin doesn't change pftools mechanism, however, it bypasses it in some places.

Comandeer

There are some failing tests:

Additionally fixture for manual test has info that it's a GDocs one.

plugins/pastefromlibreoffice/filter/default.js

Comandeer · 2019-11-15T11:57:19Z

plugins/pastefromlibreoffice/filter/default.js

+							if ( styles.color === '#000080' ) {
+								delete styles.color;
+							}
+
+							if ( styles[ 'text-decoration' ] === 'underline' ) {
+								delete styles[ 'text-decoration' ];
+							}


Maybe extract it to separate function, e.g. removeDefaultLinkStyling? At the moment it looks like some magic styles being removed.

plugins/pastefromlibreoffice/filter/default.js

Comandeer · 2019-11-15T14:00:38Z

plugins/pastetools/filter/common.js

+					if ( /%$/.test( value ) ) {
+						Style.setStyle( element, 'width', value );
+					} else {
+						Style.setStyle( element, 'width', value + 'px' );
+					}


As it's used in two places, it can be extracted to separate function.

Additionally IMO regexps should be extracted to variables with descriptive names (eg. endsWithPercentRegex).

plugins/pastetools/filter/common.js

Comandeer · 2019-11-15T14:02:13Z

plugins/pastetools/filter/common.js

+				'background:transparent',
+				'background-color:none',
+				'background:none',
+				'text-align:start',


Why is this style removed?

Probably some remaining leftover from the early stage of development.

plugins/pastetools/filter/image.js

Comandeer · 2019-11-15T14:15:53Z

tests/plugins/pastetools/filter/common.js

+			);
+		},
+
+		'test common filter should remove superflous styles': function() {


Suggested change

'test common filter should remove superflous styles': function() {

'test common filter should remove superfluous styles': function() {

I missed that during batch commit :( So I add it separately to not spend too much time on git history fixing.

msamsel · 2019-11-21T15:02:38Z

There are some failing tests:

It seems to pass on my env. Please check if those still fails after rebase to current major and if so, then provide more details on what browser and which version you are using.

jacekbogdanski · 2019-12-11T06:47:12Z

@msamsel I see that this PR introduces some changes to image handling. Could you confirm if we should wait with #2800 until this PR gets merged?

msamsel · 2019-12-11T07:39:01Z

@jacekbogdanski this PR is not related to #2800.
PFLO doesn't implement any changes to image processing in PFW. From my perspective linked issue can be handled separately, as long, as it only affects processing images. In case that proposed solution would require changes in the PFW filter, then would be better to wait for merge.

msamsel · 2020-01-09T13:21:22Z

I've rebased the pr to current major and resolve existing conflicts.

Comandeer

Overall it works really well, good job 👍.

I'm missing mainly… support for Page Break. We have it in PfW, so it would be nice to introduce it also for PfLO. It does not seem difficult, LO adds page-break-before: always style to appropriate element.

There is also an issue with aligning images.

LO:

PfLO:

On Safari image is not inserted at all. On IE8 and 11 image is inserted, but it's broken:

<img style="width: 117px; height: 117px;" src="file:///C:/Users/dev/AppData/Local/Temp/lu7500w8bapy.tmp/lu7500w8baq5_tmp_da3e8e737c4f214e.png" data-cke-saved-src="file:///C:/Users/dev/AppData/Local/Temp/lu7500w8bapy.tmp/lu7500w8baq5_tmp_da3e8e737c4f214e.png">

Most other pain points are already extracted as separate issues and my quick manual tests showed that pasting works well. So I can't even complain much 😞

plugins/pastefromlibreoffice/filter/default.js

Comandeer · 2020-01-14T12:05:00Z

plugins/pastefromlibreoffice/filter/default.js

+	}
+
+	function getListEvaluator() {
+		var guard = false;


Maybe isInBlock? Current name is not much informative.

plugins/pastefromlibreoffice/plugin.js

plugins/pastefromword/plugin.js

plugins/pastetools/filter/image.js

msamsel · 2020-01-15T11:30:07Z

Issue related to float style added to images is extracted to: #3780

msamsel · 2020-01-15T11:46:15Z

On Safari image is not inserted at all.

Safari doesn't provide the image, that's why it is not possible to obtain the image.
There is no text/rtf clipboard in the paste event and there is no src reference in text/html clipboard.

msamsel · 2020-01-15T13:04:29Z

The case related to missing images on IE11 is extracted to #3781. The image cannot be embedded in content as there is no available data which allows on that. This is why most suitable fix for this case is removing such elements, however, this change will affect the PFW behaviour.

msamsel · 2020-01-15T13:07:04Z

About IE8 I remain it as it is. PFLO is a new feature and those has limited compatibility with obsolete browsers. It doesn't look that breaks something on this browser.

…lows to avoid checking wordRegexp what reduces false positive results. Change data input to base on full text/html datatransfer which contains meta tags.

… a comment.

Comandeer · 2020-01-16T13:09:02Z

Safari doesn't provide the image, that's why it is not possible to obtain the image.

I've reported it: https://bugs.webkit.org/show_bug.cgi?id=206348

Comandeer

I'm wondering how much work it would involve to introduce support for SVG graphics (e.g. document-with-image.odt.zip; unfortunately GH does not support ODT documents). I stumbled upon this use case today. It seems that SVG file is inside RTF as pict element. Maybe we would be able to add the support easily 🤔

There are also some small things to add:

new plugin should be added to the build config in dev/builder;
there should be also info about supported environments inside the plugin.

Everything else seems to be working well 👍

msamsel · 2020-01-16T14:29:27Z

It seems that SVG file is inside RTF as pict element. Maybe we would be able to add the support easily 🤔

Unfortunately, the format of this is image is wmetafile8. It would require to write some dedicated parser/converter which would allow embedding such image in the document. It is not a clear SVG image.
When there are used png or gif images then also for each image has it's wmf representation in rtf clipboard. AFAIR I tried to analyze this format to have possibility to embed shapes drawn in MS Word unfortunately without luck.

Comandeer

There are some unit tests failing in IE8:

Some tests fail also in Edge, but it seems to be connected with their instability 🤔

Comandeer

LGTM 🎉

LiamDawe · 2020-03-05T14:46:15Z

Does this need a new toolbar button or is it added into other paste features?

f1ames · 2020-03-05T16:33:15Z

@LiamDawe - it's added by default in Standard and Full presets and doesn't use additional paste button. So all content pasted from Libre Office with Ctrl/Cmd+C, patse button or context menu will go through Libre Office filter preserving formatting.

msamsel marked this pull request as ready for review November 6, 2019 07:53

Comandeer self-requested a review November 12, 2019 12:44

Comandeer self-assigned this Nov 12, 2019

Dumluregn mentioned this pull request Nov 12, 2019

Allow user to indicate pasted content source #3646

Open

Comandeer requested changes Nov 15, 2019

View reviewed changes

CKEditorBot unassigned Comandeer Nov 15, 2019

msamsel self-assigned this Nov 18, 2019

msamsel force-pushed the t/2374 branch from 964ab8a to c0b6df7 Compare November 21, 2019 13:14

msamsel removed their assignment Nov 21, 2019

msamsel requested a review from Comandeer November 21, 2019 15:55

msamsel mentioned this pull request Jan 7, 2020

Pasting Word table made to fit window #3755

Open

msamsel force-pushed the t/2374 branch from 88c0506 to 33d0283 Compare January 9, 2020 13:20

Comandeer self-assigned this Jan 14, 2020

Comandeer requested changes Jan 14, 2020

View reviewed changes

CKEditorBot unassigned Comandeer Jan 14, 2020

msamsel self-assigned this Jan 15, 2020

msamsel mentioned this pull request Jan 15, 2020

Image pasted from libre office gets float style #3780

Open

msamsel mentioned this pull request Jan 15, 2020

How to deal with file:/// images pasted from Word or LibreOffice #3781

Open

msamsel mentioned this pull request Jan 15, 2020

Unify image processing for paste from word and libre office #3782

Closed

Mateusz Samsel added 3 commits January 15, 2020 14:59

Simplify regexps. Implement detection of meta generator tag, which al…

bc9347f

…lows to avoid checking wordRegexp what reduces false positive results. Change data input to base on full text/html datatransfer which contains meta tags.

Remove double spaces.

3869651

Add mockup for paste from libre office plugin.

f0cc898

Move priority key to be more visible and provide description to it in…

d5f0737

… a comment.

msamsel force-pushed the t/2374 branch from 446da83 to d5f0737 Compare January 15, 2020 14:00

msamsel removed their assignment Jan 15, 2020

msamsel requested a review from Comandeer January 15, 2020 14:02

Comandeer self-assigned this Jan 16, 2020

Comandeer requested changes Jan 16, 2020

View reviewed changes

CKEditorBot unassigned Comandeer Jan 16, 2020

msamsel self-assigned this Jan 16, 2020

Mateusz Samsel added 2 commits January 16, 2020 15:32

Add paste from libre office to dev/builder.

7b7fb98

Add supported environment to paste from libre office plugin.

5d6f8fe

msamsel removed their assignment Jan 16, 2020

msamsel requested a review from Comandeer January 16, 2020 14:37

Comandeer self-assigned this Jan 17, 2020

Comandeer requested changes Jan 17, 2020

View reviewed changes

CKEditorBot unassigned Comandeer Jan 17, 2020

msamsel assigned msamsel and unassigned msamsel Jan 17, 2020

Replace 'trim' with 'replace' to fix error on IE8.

ad546d7

msamsel force-pushed the t/2374 branch from 75f20ab to ad546d7 Compare January 17, 2020 15:58

msamsel requested a review from Comandeer January 17, 2020 15:59

Add changelog entry [ci skip].

034aa54

Comandeer approved these changes Jan 20, 2020

View reviewed changes

Comandeer merged commit 294ea33 into major Jan 20, 2020

CKEditorBot deleted the t/2374 branch January 20, 2020 11:46

This was referenced Jan 20, 2020

Paste from Word: option to parse OpenOffice/LibreOffice Writer inserts #2374

Closed

LibreOffice docs #3797

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce paste from libre office plugin #3624

Introduce paste from libre office plugin #3624

msamsel commented Nov 4, 2019 •

edited

Loading

Comandeer left a comment

Comandeer Nov 15, 2019

Comandeer Nov 15, 2019

Comandeer Nov 15, 2019

msamsel Nov 20, 2019

Comandeer Nov 15, 2019

msamsel Nov 20, 2019

msamsel commented Nov 21, 2019

jacekbogdanski commented Dec 11, 2019

msamsel commented Dec 11, 2019

msamsel commented Jan 9, 2020

Comandeer left a comment

Comandeer Jan 14, 2020

msamsel commented Jan 15, 2020

msamsel commented Jan 15, 2020 •

edited

Loading

msamsel commented Jan 15, 2020

msamsel commented Jan 15, 2020

Comandeer commented Jan 16, 2020

Comandeer left a comment

msamsel commented Jan 16, 2020

Comandeer left a comment

Comandeer left a comment

LiamDawe commented Mar 5, 2020

f1ames commented Mar 5, 2020

	'test common filter should remove superflous styles': function() {
	'test common filter should remove superfluous styles': function() {

Introduce paste from libre office plugin #3624

Introduce paste from libre office plugin #3624

Conversation

msamsel commented Nov 4, 2019 • edited Loading

What is the purpose of this pull request?

Does your PR contain necessary tests?

This PR contains

What is the proposed changelog entry for this pull request?

What changes did you make?

Comandeer left a comment

Choose a reason for hiding this comment

Comandeer Nov 15, 2019

Choose a reason for hiding this comment

Comandeer Nov 15, 2019

Choose a reason for hiding this comment

Comandeer Nov 15, 2019

Choose a reason for hiding this comment

msamsel Nov 20, 2019

Choose a reason for hiding this comment

Comandeer Nov 15, 2019

Choose a reason for hiding this comment

msamsel Nov 20, 2019

Choose a reason for hiding this comment

msamsel commented Nov 21, 2019

jacekbogdanski commented Dec 11, 2019

msamsel commented Dec 11, 2019

msamsel commented Jan 9, 2020

Comandeer left a comment

Choose a reason for hiding this comment

Comandeer Jan 14, 2020

Choose a reason for hiding this comment

msamsel commented Jan 15, 2020

msamsel commented Jan 15, 2020 • edited Loading

msamsel commented Jan 15, 2020

msamsel commented Jan 15, 2020

Comandeer commented Jan 16, 2020

Comandeer left a comment

Choose a reason for hiding this comment

msamsel commented Jan 16, 2020

Comandeer left a comment

Choose a reason for hiding this comment

Comandeer left a comment

Choose a reason for hiding this comment

LiamDawe commented Mar 5, 2020

f1ames commented Mar 5, 2020

msamsel commented Nov 4, 2019 •

edited

Loading

msamsel commented Jan 15, 2020 •

edited

Loading