Paste From Word (PFW) filter removes margin-top, margin-bottom

[jswiderski]

Type of report

Feature request

Provide detailed reproduction steps (if any)

1. Open attached Word file
2. Disable ACF in your editor.
3. Select All and paste into editor
   Margin Top.zip

Expected result

Spacing between lines are small just like in MS Word

Actual result

Spacing between lines are huge.

Other details

• Browser: Any
• OS: Any
• CKEditor version: 4.6+
• Installed CKEditor plugins: pastefromword

HTML seen by browser (FF in that case) contains margin-top, margin-bottom. Unfortunately the PFW removes these styles and there is no way to prevent that. This is a huge problem since there is no way to prevent that.

Editor should work in a way that non-configurable PFW allows a lot and configurable ACF can later be used to remove it. PFW should not remove styles which have significant impact on formatting.

HTML seen by browser:

<p class="MsoListParagraphCxSpFirst" style="margin-top:0cm;margin-right:1.6pt;
margin-bottom:0cm;margin-left:36.0pt;margin-bottom:.0001pt;mso-add-space:auto;
text-align:justify;text-justify:inter-ideograph;text-indent:-36.0pt;line-height:
normal;mso-list:l1 level1 lfo1"><span style="font-size:12.0pt;font-family:&quot;Arial&quot;,&quot;sans-serif&quot;;mso-fareast-font-family:
Arial;mso-bidi-font-weight:bold" lang="EN-US"><span style="mso-list:Ignore">1.<span style="font:7.0pt &quot;Times New Roman&quot;">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
</span></span></span><span dir="LTR"></span><b><span style="font-size:12.0pt;font-family:&quot;Arial&quot;,&quot;sans-serif&quot;" lang="EN-US">Test1</span></b></p>

<p class="MsoListParagraphCxSpLast" style="margin-top:0cm;margin-right:1.6pt;
margin-bottom:0cm;margin-left:36.0pt;margin-bottom:.0001pt;mso-add-space:auto;
text-align:justify;text-justify:inter-ideograph;line-height:normal"><b><span style="font-size:12.0pt;font-family:&quot;Arial&quot;,&quot;sans-serif&quot;" lang="EN-US">&nbsp;</span></b></p>

<p class="MsoNormal" style="margin-left:36.0pt;text-align:justify;text-justify:
inter-ideograph;line-height:normal"><span style="font-size:12.0pt;
font-family:&quot;Arial&quot;,&quot;sans-serif&quot;;mso-fareast-font-family:&quot;Times New Roman&quot;" lang="EN-US">Test2</span></p>

<p class="MsoListParagraphCxSpFirst" style="text-align:justify;text-justify:inter-ideograph;
text-indent:-36.0pt;line-height:normal;mso-list:l1 level1 lfo1"><span style="font-size:12.0pt;font-family:&quot;Arial&quot;,&quot;sans-serif&quot;;mso-fareast-font-family:
Arial" lang="EN-US"><span style="mso-list:Ignore">2.<span style="font:7.0pt &quot;Times New Roman&quot;">&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
</span></span></span><span dir="LTR"></span><span style="font-size:12.0pt;font-family:&quot;Arial&quot;,&quot;sans-serif&quot;" lang="EN-US">Test3</span><span style="font-size:12.0pt;font-family:&quot;Arial&quot;,&quot;sans-serif&quot;;mso-fareast-font-family:
Calibri" lang="EN-US"></span></p>

HTML seen by the editor (Notice that margin-top and margin-bottom are removed from paragraph and list):

<ol>
	<li style="margin-right:2px; text-align:justify"><span style="font-size:11pt"><span style="text-justify:inter-ideograph"><span style="line-height:normal"><span style="font-family:&quot;Calibri&quot;,&quot;sans-serif&quot;"><b><span lang="EN-US" style="font-size:12.0pt"><span style="font-family:&quot;Arial&quot;,&quot;sans-serif&quot;">Test1</span></span></b></span></span></span></span></li>
</ol>

<p style="margin-right:2px; margin-left:48px; text-align:justify">&nbsp;</p>

<p style="margin-left:48px; text-align:justify; margin-bottom:13px"><span style="font-size:11pt"><span style="text-justify:inter-ideograph"><span style="line-height:normal"><span style="font-family:&quot;Calibri&quot;,&quot;sans-serif&quot;"><span lang="EN-US" style="font-size:12.0pt"><span style="font-family:&quot;Arial&quot;,&quot;sans-serif&quot;">Test2</span></span></span></span></span></span></p>

<ol start="2">
	<li style="text-align:justify"><span style="font-size:11pt"><span style="text-justify:inter-ideograph"><span style="line-height:normal"><span style="font-family:&quot;Calibri&quot;,&quot;sans-serif&quot;"><span lang="EN-US" style="font-size:12.0pt"><span style="font-family:&quot;Arial&quot;,&quot;sans-serif&quot;">Test3</span></span></span></span></span></span></li>
</ol>

[engineering-this]

The case here is that PFW filter removes any margin which value equals 0. In some cases they might be not removed by ACF, e.g. with indent plugin. Altering this behaviour would result in Word pasted elements having redundant margin-left:0 and margin-right:0.

Reasonable workaround for this is to add config option to not remove margins that equals 0. Additionally I'm changing issue type to feature.

[jswiderski]

@f1ames and @engineering-this I’m against the configuration setting because we already have tools to remove unwanted margins and other things when pasting From MS Word - afterPasteFromWord event and ACF.

In my opinion PFW should leave all available margins by default because this is what Word passes and what browser sees. If we start getting any feedback that lines are no longer big, we can easily explain to users there is an API which allows removing these margins if they want. We can even provide code snippets for ACF configuration and afterPasteFromWord event.

Configuration setting is also bad because it only concerns 2 styles. With that approach you are taking part of the control from PFW and give it to core for very specific use case. This in my opinion also looks bad because now you have two separate entry points for filtering same content and that should not take place.

To summarize - please don't create another setting if we already have tools that do that. Instead, let’s teach our users how to use them.

[jswiderski]

I see this bug like cutting hair - you can cut it but you can't glue it back. Currently we cut out too much and can't glue it back. If we let PFW leave as much as possible and get any complaint about it (so far everyone complained about lines being too big) we can happily tell our users we have tools to cut it out even further.

[f1ames]

Altering this behaviour would result in Word pasted elements having redundant margin-left:0 and margin-right:0.

@engineering-this Is the margin-*: 0; a common style when pasting from Word? Does it occur in all/most of pasted content or only in some specific cases?

please don't create another setting if we already have tools that do that. Instead, let’s teach our users how to use them.

The reason we decide to make it a config option is that we would like to keep backwards compatibility of pasted content. Without it, after updating to new CKEditor version pasting content will start behave differently (keeping zero margins) - so if margin-*: 0; is a common style we should avoid changing its default processing.

We may change PFW to retain margin-*: 0; styles and then modify ACF to remove them by default. This way we will keep the previous behaviour and it will be easy to enable such styling by adjusting ACF (so the same approach as @jswiderski mentioned but the other way around). WDYT?

[engineering-this]

@f1ames looks quite common to me. Many tests fail when we allow margin-*:0

We may change PFW to retain margin-*: 0; styles and then modify ACF to remove them by default. This way we will keep the previous behaviour and it will be easy to enable such styling by adjusting ACF (so the same approach as @jswiderski mentioned but the other way around). WDYT?

Is there an easy way to do so? I don't see an option to add filter rule via allowedContent which would require certain value. We could do so by manually adding rules with filter.addRules, but to keep zeros user would have to disable ACF. So there would be two options: no zeros and filtering, or no filtering at all. With config option there is more: zeros could be kept or removed regardless of having ACF on or not.

[jswiderski]

I don't see an option to add filter rule via allowedContent which would require certain value.

Guys please remember that you also have afterPasteFromWord event. Allow PFW leaving the content which has significant impact on formatting. If a user ever complains about margin-*:0 we can show him a simple code snippet for afterPasteFromWord event which removes these margins. Please don't go into another - "we will be smarter in predicting what users wants". Leave the content as it is and if a user/developer complains about small lines he will be able to change that result at his own will using existing tools. Please don't do some auto-guessing about what user may or may not want. Give them result they can work with and let them choose.

The handling was bad in previous versions. Let's just fix it and if anyone doesn't like it, he will have the ability to change the result on his own because we have the tools for it already.

[f1ames]

Guys please remember that you also have afterPasteFromWord event. Allow PFW leaving the content which has significant impact on formatting.

I agree that PFW can leave those margins untouched so they can be processed for any mechanisms later in the pipeline.

If a user ever complains about margin-*:0 we can show him a simple code snippet for afterPasteFromWord event which removes these margins.

The issue here is that Word adds margin: 0; styling by default to most of the elements - if you paste simple text you have:

<p class=MsoNormal>FooBar<o:p></o:p></p>

p.MsoNormal, li.MsoNormal, div.MsoNormal {
    mso-style-unhide:no;
    mso-style-qformat:yes;
    mso-style-parent:"";
    margin:0cm;
    margin-bottom:.0001pt;
    mso-pagination:widow-orphan;
    font-size:12.0pt;
    font-family:"Calibri",sans-serif;
    mso-ascii-font-family:Calibri;
    mso-ascii-theme-font:minor-latin;
    mso-fareast-font-family:DengXian;
    mso-fareast-theme-font:minor-fareast;
    mso-hansi-font-family:Calibri;
    mso-hansi-theme-font:minor-latin;
    mso-bidi-font-family:"Times New Roman";
    mso-bidi-theme-font:minor-bidi;
}

so making margin-top/bottom: 0; inline style to appear be default is a breaking change which may break content for most of the integrations using PFW - both does relying on default browser styling and ones using their custom stylesheet (since we inline those margin styles so they have higher priority).

If a user ever complains about margin-*:0 we can show him a simple code snippet for afterPasteFromWord event which removes these margins.

As I agree that changing this behaviour should be as easy as possible (and may be outside of the PFW plugin itself), making it default behaviour is no go at the moment as it will change the way how content is displayed by default in almost all paste scenarios. So we will have to explain all users how to "revert" this breaking change instead of explaining only a few how to enable zero margins.

[f1ames]

We have agreed with @jswiderski that margin-*: 0; should be preserved by PFW filter so it can be processed further along the pasting pipeline (e.g. in afterPasteFromWord listener). From the other hand we don't want to change the default PFW output by preserving margin-*: 0; by default because it will change the way how content pasted from Word looks and may cause compatibility and styling issues (see previous #2935 (comment)).

So there are few options here:

1. Go with the initial proposal with the config option which disables margin-*: 0; removal and is turned off by default (so that margin-*: 0; is still removed by default) - but then it only partially covers the above proposal (PFW preserves margin-*: 0; depending on a config).
2. Make PFW filter always preserve margin-*: 0; and remove it later:
   • via afterPasteFromWord listener with low priority which can be prevented
   • via some config option which will be processed after PFW filters (in afterPasteFromWord low priority listener again?)
3. Any other idea?

cc @engineering-this @Comandeer

[engineering-this]

I'd go with either first option or with the opposite: config option force removal of margin-*:0 which by default will be preserved I don't think it has to be done in afterPasteFromWord, we can just keep the logic in same place it is now. I'm not sure which is better.

[jacekbogdanski]

I'm not sure if afterPasteFromWord is a good place for such processing. If we decide to preserve margin: 0 by PFW, it should be probably removed by a different plugin (like indent?). Otherwise, we are breaking plugin flow and introducing another way for HTML default processing which will make this code less maintainable (it's already pretty complicated). What about @f1ames proposition:

We may change PFW to retain margin-*: 0; styles and then modify ACF to remove them by default.

It looks for me like the best approach, although such ACF should be plugin wise (again, indent?).

[Comandeer]

I'm not sure if afterPasteFromWord is a good place for such processing. If we decide to preserve margin: 0 by PFW, it should be probably removed by a different plugin (like indent?).

AFAIR there was some unofficial rule that "PFW changes only things that are changeable by other means (plugins)". Probably in long term we should enhance the currently selected solution with introducing some kind of plugin to control the margins.

In case of ad-hoc solution I'm for config option.

[f1ames]

AFAIR there was some unofficial rule that "PFW changes only things that are changeable by other means (plugins)". Probably in long term we should enhance the currently selected solution with introducing some kind of plugin to control the margins.

Makes sense because manipulating attributes/styling which cannot be adjusted in any way leaves users in a quite uncomfortable situation. Still, here the user is not able to add removed margin (and will not be able to remove it if it is preserved - unless in source mode or maybe with remove format plugin).

In case of ad-hoc solution I'm for config option.

Yes, I think we could proceed with config as proposed in initial PR and then improve this solution if necessary.

---

We may change PFW to retain margin-*: 0; styles and then modify ACF to remove them by default.

It looks for me like the best approach, although such ACF should be plugin wise (again, indent?).

It's about margin-top/bottom and the issue here is that there is no plugin which can manage/manipulate this values (with such plugin we would be fine with just preserving mentioned margins in PFW because the plugin itself will take care of it).