Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#2125 closed Bug (fixed)

InsertHtml() ignores current selection in IE. Again.

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


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 martinkou 9 years ago.
fck_file.js (5.1 KB) - added by alfonsoml 9 years ago.
fixed version of the plugin

Download all attachments as: .zip

Change History (9)

comment:1 Changed 9 years ago by martinkou

  • Keywords Confirmed IE added
  • Owner set to martinkou
  • Status changed from new to assigned

comment:2 Changed 9 years ago by martinkou

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 9 years ago by martinkou

comment:3 Changed 9 years ago by martinkou

  • Keywords Review? added

comment:4 Changed 9 years ago by alfonsoml

  • Summary changed from The #1990 bug is back again. to 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 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 9 years ago by alfonsoml

fixed version of the plugin

comment:5 Changed 9 years ago by fredck

  • 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 9 years ago by martinkou

  • Resolution set to fixed
  • Status changed from assigned to closed

Fixed with [1963].

Click here for more info about our SVN system.

comment:7 Changed 9 years ago by alfonsoml

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 – 2016 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy