Opened 9 years ago
Closed 9 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.
- Clear editor contents
- Type "test test"
- Press Ctrl+A
- 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 9 years ago by
Status: | new → confirmed |
---|
comment:2 Changed 9 years ago by
Milestone: | → CKEditor 4.5.3 |
---|
comment:3 Changed 9 years ago by
Owner: | set to Szymon Cofalik |
---|---|
Status: | confirmed → assigned |
comment:4 Changed 9 years ago by
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.
comment:5 Changed 9 years ago by
Status: | assigned → review |
---|
comment:6 Changed 9 years ago by
Resolution: | → fixed |
---|---|
Status: | review → closed |
Fixed on master with git:239db6d. Great job!
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.