Opened 11 years ago
Last modified 11 years ago
#12750 reopened Bug
Paste from Word: strikethrough and underscore should have the same color as font
| Reported by: | Piotr Jasiun | Owned by: | Piotr Jasiun |
|---|---|---|---|
| Priority: | Normal | Milestone: | |
| Component: | General | Version: | |
| Keywords: | Cc: |
Description
Maintain color of strikethrough and underscore the same as font color of the text when copying from Word into CKEditor.
TC:
- In MS Word, type in “Test font” with font family = Calibri, size = 14px and font color = red and then underscore and strikethrough the words “Test font”.
- Copy from Word and paste the same in CK Editor. The font color remains red as expected but the underscore and strikethrough changes to default black.
Solution: the color of strikethrough (<s>) and underline (<u>) will be maintained, but only if colors, underline, strikethrough are applied to exactly the same selection (portion of text).
Attachments (2)
Change History (13)
comment:1 Changed 11 years ago by
| Owner: | set to Piotr Jasiun |
|---|---|
| Status: | new → assigned |
comment:2 Changed 11 years ago by
| Status: | assigned → review |
|---|
comment:3 follow-up: 4 Changed 11 years ago by
| Status: | review → review_failed |
|---|
The patch is amazing. Due to the nature of the MSWord filter, I expected something much uglier. Kudos for finding so simple solution.
The condition needs some improvements:
hasNotTextChild->hasNoTextChild- The
hasNoTextChildvariable actually means more, because the element also has to have only one child, - But the above is not important, because instead of checking:
hasManyChildren || hasOneNonTextChild
I think we could check the opposite:
!( children.length == 0 || children.length == 1 && children[ 0 ].type == TEXT )
in other words - we don't want to move an element if:
isEmpty || hasTextOnly
I find this condition much clearer (and the variable names make sense :D).
Other:
element.moved->element._.moved- Tests fail on IEs (I checked 8, 11) because of styles comparison problems. Avoid hex colors and
.0values.
comment:4 Changed 11 years ago by
Replying to Reinmar:
hasNotTextChild->hasNoTextChild- The
hasNoTextChildvariable actually means more, because the element also has to have only one child,
This is why this variable is called hasNotTextChild instead of hasNoTextChild: it means that element has child and the type of the child is "not text". Anyway the name of variable is definitely bad if I need to explain it so I will change.
comment:5 Changed 11 years ago by
| Status: | review_failed → review |
|---|
I refactored moveToText and fixed tests. Changes in t/12750b.
comment:6 Changed 11 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | review → closed |
Fixed on master with git:a5f8c73.
comment:7 Changed 11 years ago by
| Milestone: | → CKEditor 4.4.7 |
|---|
Changed 11 years ago by
| Attachment: | 12750.docx added |
|---|
Changed 11 years ago by
comment:8 Changed 11 years ago by
The attached document gave me
<p><s><u><span style="color:red; font-size:14pt">Test font</span></u></s></p>
Word 2010, IE11 / Win 7.
comment:9 Changed 11 years ago by
Uh, forget my last comment, It turned out that during pasting "Paste from Word" was skipped - reported in #12765.
When using the "Paste from Word" button everything worked as expected.
comment:10 Changed 11 years ago by
| Milestone: | CKEditor 4.4.7 |
|---|---|
| Resolution: | fixed |
| Status: | closed → reopened |
We're struggling with #12842 - a regression introduced by this fix. Since it's unlikely that with the current PFW implementation we'll be able to fix #12842 in a clean way I decided to revert #12750. We may still eventually close both tickets, but slightly better solution to #12842 needs to be found than the one we have now
comment:11 Changed 11 years ago by
I pushed branch:t/12750 with the current state from the point at which we previously merged it to master.
I also pushed branch:t/12750b with the fix proposed for #12842 (in the comment no. 5).


I have implemented filter which move strike and underscore elements close to the child text nodes. Works in all tested cases. Changes in t/12750b.