Opened 10 years ago
Closed 10 years ago
#12617 closed Bug (fixed)
uploadWidget.replaceWith should use insertHtmlIntoRange
Reported by: | Piotr Jasiun | Owned by: | Piotr Jasiun |
---|---|---|---|
Priority: | Normal | Milestone: | CKEditor 4.5.0 Beta |
Component: | General | Version: | |
Keywords: | Cc: |
Description
uploadWidget.replaceWith
should use insertHtmlIntoRange
and support multiple elements.
Change History (6)
comment:1 Changed 10 years ago by
Status: | new → confirmed |
---|
comment:2 Changed 10 years ago by
Owner: | set to Piotr Jasiun |
---|---|
Status: | confirmed → assigned |
comment:3 Changed 10 years ago by
Status: | assigned → review |
---|
comment:4 Changed 10 years ago by
Status: | review → review_failed |
---|
I rebased the branch and added new tests. They fail. When operating over DOM with editable.insertHtmlIntoRange you must remember that selection may be lost so it must be always restored. I think that it will be enough to create bookmark before calling insertHtmlIntoRange and restoring it afterwards (both should only be done if wasSelected == false).
Moreover, there's no test for preserving widget selection (if wasSelected works).
Moreover, I understood one sad thing. Since we use insertHtmlIntoRange with the default mode it will always work like pasting, so inserting "foo" in <b>xx^xx</b>
will result with <b>xx</b>foo<b>xx</b>
. In some cases this will be undesired behaviour. I think that we can solve this by adding the mode param to the replaceWith method. You can extract this to a new ticket.
comment:5 Changed 10 years ago by
Status: | review_failed → review |
---|
I have improved handling selection (code and test) and added mode
parameter. Changes in t/12617.
comment:6 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | review → closed |
It works ok. Not perfectly, because e.g. selection direction is lost (unfortunately, not much we can do about this) and on FF the image selection isn't visible (dunno why), but most important cases work.
Merged to major with git:b4b3a83.
Integration seems to work smoothly. Changes in t/12617.