Opened 3 years ago

Closed 3 years ago

#13568 closed Bug (fixed)

Method getSelectedHtml() seems to return invalid results.

Reported by: Jakub Ś Owned by: Szymon Cofalik
Priority: Normal Milestone: CKEditor 4.5.3
Component: General Version: 4.5.0
Keywords: Support Cc:

Description

This issue has been reported on our support channel.

  1. Clear editor contents
  2. Type "test test"
  3. Press Ctrl+A
  4. Fire e.g. in Firebug or IE or Chrome console
    console.log(editor.getSelection().getSelectedText());
    console.log(editor.getSelectedHtml(true));
    console.log(editor.getSelectedHtml().getHtml());
    

Results:
IE11 and FF:

test test
test test<p></p>
test test<p></p>

IE8, Safari and Blink:

test test
test test
test test

IE9-10:

test test 
<p>test test</p> 
<p>test test</p> 

IMHO only IE9-10 seem to return correct results.

Change History (6)

comment:1 Changed 3 years ago by Jakub Ś

Status: newconfirmed

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

Milestone: CKEditor 4.5.3

Only the results on Firefox and IE11 are wrong. The rest is ok. There will be difference between browsers because of difference in selection systems.

Last edited 3 years ago by Piotrek Koszuliński (previous) (diff)

comment:3 Changed 3 years ago by Szymon Cofalik

Owner: set to Szymon Cofalik
Status: confirmedassigned

comment:4 Changed 3 years ago by Szymon Cofalik

I pushed branch:t/13568 with a fix and a test.

There was an oversight in execContentsAction() algorithm that was causing a bug in a special case. The bug was concerning only browsers that appended bogus br to the p. We had code similiar to:

<html>
 <body>
  <p>
   Foo bar
   <br />
  </p>
 </body>
</html>

And the range was from body#0 to p#1. In execContentsAction() we build startParents and endParents arrays. p was shared node in both arrays and it was the node pointed by minLevel so it was processed in "left branch loop". Then levelParent was set back to docFrag. After that, p node was attempted to process in "right branch loop" but the algorithm noticed that it was already processed so it skiped that one loop. But then the endNode (Foo bar) was appended to levelParent that was still set to docFrag.

The solution was to updated levelParent when we detect that a node was already processed in "left branch loop".

Edit: I also removed a block of code that was signed as redundant. From what I analyzed, it seems it really was.

Last edited 3 years ago by Szymon Cofalik (previous) (diff)

comment:5 Changed 3 years ago by Szymon Cofalik

Status: assignedreview

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

Resolution: fixed
Status: reviewclosed

Fixed on master with git:239db6d. Great job!

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