Opened 7 years ago

Closed 7 years ago

Last modified 6 years ago

#9709 closed Bug (fixed)

pastefromword should not depend on configuration from other styles

Reported by: Garry Yao Owned by: Piotrek Koszuliński
Priority: Normal Milestone: CKEditor 4.0.1
Component: Plugin : Paste from Word Version: 4.0 Beta
Keywords: Cc:

Description

Paste from word read the configuration on editor styles (block and inline) to properly migrate the elements, but it should still work without those plugins.

Change History (11)

comment:1 Changed 7 years ago by Garry Yao

Owner: set to Garry Yao
Status: newreview
Version: 4.0 (GitHub - master)4.0 Beta

Opened t/9709 for review.

comment:2 Changed 7 years ago by Piotrek Koszuliński

I moved branch from old cksource remote to Github: git:eb9371d.

comment:3 Changed 7 years ago by Piotrek Koszuliński

Status: reviewreview_failed

Pastefromword depends also on basicstyles, font and colobutton.

https://github.com/ckeditor/ckeditor-dev/blob/master/plugins/pastefromword/filter/default.js#L916-L933

HTML to verify format dependency:

<h1 class="MsoNormal">xxx</h1>

HTML to verify basicstyles dependency:

<p class="MsoNormal">b<b>b</b>b</p>

<p>xxx</p>

HTML to verify font dependency (config.pasteFromWordRemoveFontStyles has to be set to false):

<p class="MsoNormal">x<span style="font-family:Arial">x</span>x</p>

<p>xxx</p>

HTML to verify colorbutton dependency (config.pasteFromWordRemoveFontStyles has to be set to false):

<p class="MsoNormal">x<span style="color:red">x</span>x</p>

<p>xxx</p>

I was copy&pasting these on Chrome. Other browsers may not preserve styles.

comment:4 Changed 7 years ago by Piotrek Koszuliński

I forgot to mention why I R-ed this branch :D.

The idea proposed in ​git:eb9371d is good and would help if there hasn't been an error. There's an old typo in variable name - styleDefiniton.

Also, I think that this issue deserves tests. Testing pasteFromWord is pretty simple having code samples I attached in previous comment and assertPasteEvent function with improvements for async execution from #9456.

Please, also try to avoid too much conflicts with tests for #8893.

comment:5 Changed 7 years ago by Garry Yao

Status: review_failedreview

Forced update the typo fixes with a624914

Opened t/9709 for style related tests.

comment:6 Changed 7 years ago by Piotrek Koszuliński

Status: reviewreview_failed

http://ckeditor4.t/dt/plugins/pastefromword/keep_styles.html is failing on IE9 because of spaces inside style attributes.

I pushed untracked file that was added by #9456.

comment:7 Changed 7 years ago by Piotrek Koszuliński

Owner: changed from Garry Yao to Piotrek Koszuliński
Status: review_failedassigned

comment:8 Changed 7 years ago by Piotrek Koszuliński

Status: assignedreview

I decided that for now we can only use compatHtml - it makes comparison a little bit senseless, because it fixes a lot of issues, but we don't have better (highly configurable) parser&writer duet.

Pushed few fixes to t/9709@tests.

comment:9 Changed 7 years ago by Olek Nowodziński

Status: reviewreview_passed

comment:10 Changed 7 years ago by Piotrek Koszuliński

Resolution: fixed
Status: review_passedclosed

Masterised git:cac827d.

comment:11 Changed 6 years ago by Frederico Caldeira Knabben

Component: Core : PastingPlugin : Paste from Word
Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy