Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#2125 closed Bug (fixed)

InsertHtml() ignores current selection in IE. Again.

Reported by: Mark Bryson Owned by: Martin Kou
Priority: Normal Milestone: FCKeditor 2.6.1
Component: UI : Dialogs Version: SVN (FCKeditor) - Retired
Keywords: Confirmed IE Review+ Cc:

Description

I hate to be a pest about the same thing over and over again, but the bug as reported in #1990 is back as originally stated for both 2.6 and the SVN.

For whatever reason, the condition put in place to fix #2057, which was the fix for the flashing problem of the fix for #1990, that detects whether the selection has already been restored is always true when running the simple plugin attached to #1990. Therefore, the selection never does get restored hence why the original #1990 problem.

Perhaps the logic implemented by #2057 will only work if the EditorDocument doesn't have focus. This is my guess because if I rearrange lines 152 thru 155 in the InsertHtml function of 'fck_ie.js' such that FCKUndo.SaveUndoStep() happens before setting focus:

	FCKUndo.SaveUndoStep() ;

//	FCK.Focus() ;
	FCK.EditorWindow.focus() ;

that seems to fix it. Perhaps logic of #2057 should be reviewed if needed for other broken circumstances. Putting that save undo call before the InsertHtml() call in the plugin also works around it. (What fun could you have without IE?)

FYI... The FCKSelection.Restore() function gets called a lot, even when there is no active dialog. Not much can be done with the editor per 100 times this gets called. However, the call rate gets drastically reduced after a positive response (e.g. OK button) to a dialog.

Attachments (2)

2125.patch (1.0 KB) - added by Martin Kou 10 years ago.
fck_file.js (5.1 KB) - added by Alfonso Martínez de Lizarrondo 10 years ago.
fixed version of the plugin

Download all attachments as: .zip

Change History (9)

comment:1 Changed 10 years ago by Martin Kou

Keywords: Confirmed IE added
Owner: set to Martin Kou
Status: newassigned

comment:2 Changed 10 years ago by Martin Kou

You're right in your guess - InsertHtml doesn't because Restore() thinks the current selection is already inside the editor window and so it didn't bother. Moving SaveUndoStep() before the focus() call solves the problem because SaveUndoStep() calls FCKSelection.Restore() - but that's a hack which might break in the future if we have changed SaveUndoStep().

I guess a better solution would be calling FCKSelection.Restore() before focus(). The focus() call is still needed in case the user selected something outside of the editor without opening a dialog.

Changed 10 years ago by Martin Kou

Attachment: 2125.patch added

comment:3 Changed 10 years ago by Martin Kou

Keywords: Review? added

comment:4 Changed 10 years ago by Alfonso Martínez de Lizarrondo

Summary: The #1990 bug is back again.InsertHtml() ignores current selection in IE. Again.

That patch doesn't seems to work in all the situations.

I don't remember if the source files in http://martinezdelizarrondo.com/easyupload/ do include this patch, but I know that locally I've tested it in order to fix the bug in the Insert Attachment dialog, and it didn't work. You can install the plugin to test it.

The logic of the dialog is a little convoluted: When the user presses OK, I start the file upload and then cancel the OK command. When the file has been uploaded then I call the window.parent.Ok() myself in order to properly close the dialog, but then the new link is placed at the beginning.

I've fixed it by calling window.focus(), I'll upload now the changed file so you can check it and you can decide if it's worth study better why it doesn't work without that workaround.

Changed 10 years ago by Alfonso Martínez de Lizarrondo

Attachment: fck_file.js added

fixed version of the plugin

comment:5 Changed 10 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

Martin's comments makes sense, and the patch seems ok, for the reported bug at least.

@alfonsoml, your problem may be related to something else. I've tried to debug it, but I'm having permission denied in the OnUploadCompleted call when running your plugin. I would recommend opening a dedicated ticket for it.

comment:6 Changed 10 years ago by Martin Kou

Resolution: fixed
Status: assignedclosed

Fixed with [1963].

Click here for more info about our SVN system.

comment:7 Changed 10 years ago by Alfonso Martínez de Lizarrondo

The OnUploadCompleted is a bug in the quickupload since the domain relaxation: #2115

I've checked that reverting [1818] does fix the focus problem that I've mentioned, so I'll add this info to #2126 as that bug is already handling another regression due to #2057

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