Opened 2 years ago

Last modified 2 years ago

#16618 review_failed Bug

[IE11][FF][CF] Styling could not be reapplied on a simple click

Reported by: kkrzton Owned by: kkrzton
Priority: Normal Milestone: CKEditor 4.7.1
Component: General Version: 4.6.0
Keywords: Cc:

Description (last modified by kkrzton)

Steps to reproduce

Can be easily reproduced on /tests/plugins/copyformatting/manual/safaricolor (for both editors).

  1. Put caret inside first paragraph e.g. Text witho^ut any format.
  2. Click Copy Formatting button.
  3. Click on the last word (Orange) inside last paragraph (styling will be removed).
  4. Click on the still styled word inside same paragraph e.g. Text with Bac^kground Colour Orange.
  5. Click Copy Formatting button.
  6. Click on the last word (Orange) inside last paragraph.

Expected result

On the second click styling should be copied, so word Orange should be styled same as rest of the paragraph.

Actual result

On the second click, styling is not applied.

Other details (browser, OS, CKEditor version, installed plugins)

IE11, FF (Chrome works as expected).

Change History (14)

comment:1 Changed 2 years ago by kkrzton

Status: newconfirmed

comment:2 Changed 2 years ago by kkrzton

Description: modified (diff)
Summary: [IE11][CF] Styling could not be reapplied on a simple click[IE11][FF][CF] Styling could not be reapplied on a simple click

comment:3 Changed 2 years ago by Marek Lewandowski

Milestone: CKEditor 4.6.1

comment:4 Changed 2 years ago by kkrzton

Owner: set to kkrzton
Status: confirmedassigned

comment:5 Changed 2 years ago by kkrzton

Applying styles for the first time (step 3) produces html like (on Firefox):

<p>
    <span style="background-color: #ff8c00;">Text with Background Colour </span>
    Orange
    <span style="background-color: #ff8c00;"></span>
    <br type="_moz">
</p>

In other browsers there is only empty span on the end, here is additional <br>. Those two empty elements causes some error in _getSelectedWordOffset function. With one empty element on the end, it works fine.

comment:6 Changed 2 years ago by kkrzton

Pushed unit tests for this particular case ec3cf00.

comment:7 Changed 2 years ago by kkrzton

Found similar issue but with different cause, extracted to #16718.

comment:8 Changed 2 years ago by kkrzton

Status: assignedreview

Pushed fix to t/16618 branch.

The code assumed that every (every used in a particular method) element node has a text child which was false for e.g. br nodes. I added an additional check.

comment:9 Changed 2 years ago by Tomasz Jakut

Priority: NormalMust have (possibly next milestone)
Status: reviewreview_failed

The fix is not working correctly. It works only for empty span + br at the boundary of p. If the sentence is not inside paragraph, the plugin apply formatting to the wrong word.

Also only test applyFormat ending with empty span and br - #16618 really fails after disabling the fix.

Additionally test applyFormat ending with br - #16618 passes even after disabling fix in FF.

comment:10 Changed 2 years ago by Marek Lewandowski

Priority: Must have (possibly next milestone)Normal

comment:11 Changed 2 years ago by kkrzton

I can see that the fix does not work for some specific cases mentioned above.

Pushed additional tests covering those cases. Did some tests refactoring - now all tests but 2 fails on Firefox. These 2 tests just covers before untested case with space between text node and empty elements.

comment:12 Changed 2 years ago by Marek Lewandowski

Milestone: CKEditor 4.6.1CKEditor 4.6.2

Let's focus on bringing it into 4.6.2.

comment:13 Changed 2 years ago by Marek Lewandowski

Milestone: CKEditor 4.6.2CKEditor 4.7.0

comment:14 Changed 2 years ago by Marek Lewandowski

Milestone: CKEditor 4.7.0CKEditor 4.7.1
Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy