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

Paste from Libre Office changes the behaviour of paste filter #3809

Open
Comandeer opened this issue Jan 22, 2020 · 9 comments
Open

Paste from Libre Office changes the behaviour of paste filter #3809

Comandeer opened this issue Jan 22, 2020 · 9 comments
Labels
browser:chrome The issue can only be reproduced in the Chrome browser. browser:safari The issue can only be reproduced in the Safari browser. plugin:pastefromlibreoffice The plugin which probably causes the issue. status:confirmed An issue confirmed by the development team. type:bug A bug.
Milestone

Comments

@Comandeer
Copy link
Member

Type of report

Bug

Provide detailed reproduction steps (if any)

  1. Open https://codepen.io/Comandeer/pen/MWYZZMN
  2. Copy styled span.
  3. Open local CKEditor sample on major branch.
  4. Paste copied span.

Expected result

Styles are not preserved (you can see expected result by pasting into the editor inside the pen).

Actual result

Background and font color are preserved.

Other details

In Chrome and Safari paste filter is set by default to semantic-content, to tidy the polluted HTML produced by these browsers. However the inability to detect the source of paste in Safari and IE resulted in the general filter in PfLO, which changes the behaviour of default paste filter in Blink and WebKit.

  • Browser: Chrome, Safari
  • CKEditor version: major
  • Installed CKEditor plugins: pastefromlibreoffice
@Comandeer Comandeer added type:bug A bug. browser:chrome The issue can only be reproduced in the Chrome browser. browser:safari The issue can only be reproduced in the Safari browser. plugin:pastefromlibreoffice The plugin which probably causes the issue. labels Jan 22, 2020
@f1ames
Copy link
Contributor

f1ames commented Jan 22, 2020

It looks like a serious regression 😭

From what I understand, we are not able to differentiate content pasted from Libre Office in Safari and IE. The semantic-content filter on Chrome/Safari is overwritten by PfLO or there is some logic which just "skips" semantic-content filter (what's the order of things here 🤔).

Questions:

  1. Why the PfLO filter also kicks in in Chrome if it was suppose to solve the issue only for Safari/IE?
  2. What can we do about Safari?

🤔 TBH looks like a hard case since we are not able to distinguish from what source the content was pasted...

@f1ames f1ames added this to the Iteration 2020-1 (4.14.0) milestone Jan 22, 2020
@msamsel
Copy link
Contributor

msamsel commented Jan 23, 2020

  1. Why the PfLO filter also kicks in in Chrome if it was suppose to solve the issue only for Safari/IE?

The check is not browser specific. We could narrow down the scope of the issue, but still the problematic scenario might appear on Safari and IE:

// The filter will be run also for a regular content, as there is no way to detect apropriate source under IE11 and Safari.
return generatorName ? generatorName === 'libreoffice' : true;

  1. What can we do about Safari?

Without meta information in clipboard it won't be possible to distinguish content copied from website from content copied from Libre Office :(
We could apply paste filter in such cases anyway, by making flag dontFilter browser related:

data.dontFilter = true;

However this might result with stripping styles pasted from libre office when those are not defined in pasteFilter.

@f1ames
Copy link
Contributor

f1ames commented Jan 24, 2020

The check is not browser specific. We could narrow down the scope of the issue, but still the problematic scenario might appear on Safari and IE

So theoretically we could narrow it down to Safari and IE11 (and older IEs probably) 🤔 For now canHandle function always returns true for any text/html content which doesn't have generator meta tag:

var data = evt.data,
textHtml = data.dataTransfer.getData( 'text/html', true ) || data.dataValue,
generatorName;
// Do not run filter if there is no input data.
if ( !textHtml ) {
return false;
}
generatorName = CKEDITOR.plugins.pastetools.getContentGeneratorName( textHtml );
// The filter will be run also for a regular content, as there is no way to detect apropriate source under IE11 and Safari.
return generatorName ? generatorName === 'libreoffice' : true;

which might result on cases like the above 🤔 So narrowing it down seems like resolving the issue at least on some browsers.

Without meta information in clipboard it won't be possible to distinguish content copied from website from content copied from Libre Office :(

So it means we couldn't properly handle it on Safari? 😨 And we have to choose between handling correctly Libre Office content and regression to filtering? 😬

@msamsel
Copy link
Contributor

msamsel commented Jan 24, 2020

So it means we couldn't properly handle it on Safari? 😨 And we have to choose between handling correctly Libre Office content and regression to filtering? 😬

Yes. Take a look at clipboard data obtained on Safari for a content with a simple one paragraph:

<p style="margin-bottom: 0in; line-height: 16px; background-color: transparent; caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); font-family: -webkit-standard; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; text-decoration: none; background-position: initial initial; background-repeat: initial initial;">Simple text</p>

Content looks like a regular paragraph with a bunch of attributs. None of them seems to be LibreOffice specific, what could allow to distinguish this content from regular copy-paste of an html data.

For Chrome data contains head section with meta informations, where LibreOffice is clearly defined as content genertor.

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN">
<html>
<head>
<meta http-equiv="content-type" content="text/html; charset=utf-8"/>
<title></title>
<meta name="generator" content="LibreOffice 6.3.2.2 (MacOSX)"/>
<style type="text/css">
@page { size: 8.27in 11.69in; margin: 0.79in }
p { margin-bottom: 0.1in; line-height: 115%; background: transparent }
</style>
</head>
<body lang="en-US" link="#000080" vlink="#800000" dir="ltr"><p style="margin-bottom: 0in; line-height: 100%">
Simple text</p>
</body>
</html>

@f1ames
Copy link
Contributor

f1ames commented Jan 28, 2020

Ok, my proposal is as follows:

  1. Narrow the check to Safari and IE only as proposed in Paste from Libre Office changes the behaviour of paste filter #3809 (comment).
  2. To not introduce regressions, on Safari we should stick with semantic-content filter (so it should work as before, filtering spans from Libre Office).
  3. Mention in the docs that this is a browser limitation that is hard to workaround and that it works like that by design.

WDYT @Comandeer? @jacekbogdanski any thought on this one?

@jacekbogdanski
Copy link
Member

jacekbogdanski commented Jan 28, 2020

Not sure, you probably wrote the same, but if it's only Safari and IE11 (older?) problem - maybe let's just disable PFLO feature for these browsers? We can go with some deeper research later if we can introduce PFLO without regressions for these browsers. We can even treat #3646 as a replacement/workaround.

@Comandeer
Copy link
Member Author

Both PfLO and PfW explicitly disable paste filter:

data.dontFilter = true;

If we're going to use semantic-content, we would probably need to do it manually, right inside PfLO.

What's more, switching on filter produces very poor results.

Libre Office document:

Content pasted in Chrome with paste filter switched on:

We can go with some deeper research later if we can introduce PFLO without regressions for these browsers

I doubt it. Safari won't expose any metadata (it was already a case in GDocs → #3257 (comment)) and LO produces too clean HTML to be able to detect it.

@f1ames
Copy link
Contributor

f1ames commented Jan 28, 2020

@Comandeer @jacekbogdanski so we basically have a choice between disabling PfLO completely for Safari (as @Comandeer mentioned it's highly improbable something will change in Safari) or go with poor version due to how Safari internally works (mentioning this case in the docs).

Eventually, we could go with #3646 but I doubt we will be able to cover it in a foreseeable future.

@f1ames
Copy link
Contributor

f1ames commented Jan 28, 2020

After F2F talk with @jacekbogdanski and @Comandeer we have decided that PfLO should be disabled for Safari for now and we will enable it whenever #3646 will be covered.

Leaving this issue open for future reference.

@jacekbogdanski jacekbogdanski added the status:confirmed An issue confirmed by the development team. label Jan 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browser:chrome The issue can only be reproduced in the Chrome browser. browser:safari The issue can only be reproduced in the Safari browser. plugin:pastefromlibreoffice The plugin which probably causes the issue. status:confirmed An issue confirmed by the development team. type:bug A bug.
Projects
None yet
Development

No branches or pull requests

4 participants