Opened 16 years ago

Closed 16 years ago

Last modified 16 years ago

#1990 closed Bug (fixed)

InsertHtml() ignores selection from floating dialog in IE7

Reported by: Mark Bryson Owned by: Frederico Caldeira Knabben
Priority: Normal Milestone: FCKeditor 2.6
Component: UI : Dialogs Version: SVN (FCKeditor) - Retired
Keywords: Confirmed Review+ Cc:

Description

"FCK.InsertHtml()" function ignores selection/position when executed from a floating dialog instanciated with "FCKDialogCommand", and the web browser is IE7. The inserted text always gets inserted at the beginning. This does NOT happen when using Firefox 2.0.0.12. Furthermore, it does not happen if executed from an IE popup using window.showModalDialog().

Attached is a very simple plugin called 'insertbug' that adds the command 'InsertText' for demonstrating the problem. Firefox has no problem with selected text getting replaced or inserted at the insertion carot position. With IE, "Inserted Text" is always inserted at the beginning.

Tested 2.6.beta and nightly build 18335 (03/09/2008).

Attachments (3)

insertbug.zip (1.4 KB) - added by Mark Bryson 16 years ago.
plugin for duplication
1990.patch (20.4 KB) - added by Frederico Caldeira Knabben 16 years ago.
1990_2.patch (20.5 KB) - added by Frederico Caldeira Knabben 16 years ago.

Download all attachments as: .zip

Change History (14)

Changed 16 years ago by Mark Bryson

Attachment: insertbug.zip added

plugin for duplication

comment:1 Changed 16 years ago by Martin Kou

Owner: set to Martin Kou
Status: newassigned

comment:2 Changed 16 years ago by Martin Kou

Keywords: Discussion added

We've actually got an API added in v2.6 to deal with the problem. It is a known problem that IE cannot keep multiple selections across iframes, and so we've been saving the last known selection position inside the editor iframe just before the dialog opened. You can restore the last selection position for InsertHtml() by calling window.parent.Selection.EnsureSelection() before InsertHtml(), which is what we're doing in the 2.6 dialogs.

Your ticket leads to a possible improvement upon the current implementation however - plugin developers probably don't care or don't know about the IE bug I've mentioned and the EnsureSelection() workaround. Maybe we should call the EnsureSelection() workaround in InsertHtml() and other content manipulating functions in our API to make it transparent to plugin developers.

comment:3 Changed 16 years ago by Mark Bryson

I did find the API you speak of after I submitted this, but it took some digging. It would be nice if cross-browser consistency is maintained for the 'documented' user API. However, documentation could also work, while such an API could hide what developers will ultimately be needing to know anyway for other reasons. If 'other reasons' could possibly involve development of plugins, I think I'd go with documentation. Keep up the good work. It looks to me like you'll end up making a sensible decision.

comment:4 Changed 16 years ago by Frederico Caldeira Knabben

Keywords: Confirmed added; Discussion removed

Martin, you proposal for it makes a lot of sense. Let's make it transparent, doing any kind of fix in this sense inside the API.

We could have the FCK.SaveSelection() and FCK.EnsureSelection() functions (empty for non-IE), which would then be used in the FCKDialog and the API, for those functions that are usually called by dialogs, and that depend on the selection, like InsertHtml.

At this point, EnsureSelection should be removed from fckdialog.html, as well as any unnecessary calls to it.

comment:5 Changed 16 years ago by Frederico Caldeira Knabben

Owner: changed from Martin Kou to Frederico Caldeira Knabben
Status: assignednew

comment:6 Changed 16 years ago by Frederico Caldeira Knabben

Status: newassigned

Changed 16 years ago by Frederico Caldeira Knabben

Attachment: 1990.patch added

comment:7 Changed 16 years ago by Frederico Caldeira Knabben

Keywords: Review? added

The provided patch brings a huge change to the code, mainly for the points dealing with selection. Basically now, using FCKSelection is a safe way to be sure the selection will be in the right place for IE. It should bring backward compatibility with legacy plugins.

Another thing introduced is "FCK.IsSelectionChangeLocked", making it possible to stop firing OnSelectionChange. This was necessary because I was having infinite loops when restoring the selection. It should also bring some performance benefits, opening the possibility of using it in another place, like the Enter Key handler, as already proposed in another ticket.

Changed 16 years ago by Frederico Caldeira Knabben

Attachment: 1990_2.patch added

comment:8 Changed 16 years ago by Frederico Caldeira Knabben

I've attached a new patch, following Alfonso's findings reported on IRC.

comment:9 Changed 16 years ago by Martin Kou

Keywords: Review+ added; Review? removed

A very nice patch. Passes my regression tests (e.g. #2024, #1801, and one more on control selection which I forgot the ticket #..) with flying colors as well. Review+.

comment:10 Changed 16 years ago by Frederico Caldeira Knabben

Resolution: fixed
Status: assignedclosed

Fixed with [1795]. Click here for more info about our SVN system.

comment:11 Changed 16 years ago by Alfonso Martínez de Lizarrondo

This patch has caused #2495

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy