Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#7932 closed Bug (fixed)

Focus jump to top when reentering the iframe (in IE8)

Reported by: Ulrich Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.6.3
Component: Core : Selection Version: 3.1
Keywords: IE Cc: gogobu@…, ruud@…, mattleff@…

Description (last modified by Frederico Caldeira Knabben)

Steps to reproduce :

  • Open demo in IE8
  • Delete some text, so that the body is smaller than the iframe
  • Place Focus in the text somewhere
  • Click outside of the editor
  • Click in the white space in the iframe below the body

Focus jumps to position 1, but should be at the end

Browser name and OS : IE8 on Windows XP

Attachments (8)

7932.patch (3.8 KB) - added by Garry Yao 6 years ago.
7932_2.patch (5.7 KB) - added by Garry Yao 6 years ago.
7932_3.2.patch (5.8 KB) - added by Garry Yao 6 years ago.
7932_4.patch (5.8 KB) - added by Garry Yao 6 years ago.
review.patch (5.8 KB) - added by Garry Yao 6 years ago.
7932_3 (5.8 KB) - added by Garry Yao 6 years ago.
7932_3.patch (5.5 KB) - added by Garry Yao 6 years ago.
7932_5.patch (6.7 KB) - added by Garry Yao 6 years ago.

Download all attachments as: .zip

Change History (39)

comment:1 Changed 6 years ago by Jakub Ś

Status: newconfirmed
Version: 3.6.1 (SVN - trunk)3.0

This is rather browser dependable. If we can't do anything about it please close the issue.

Opera remembers the position.

IE places cursor at the beginning.

FF and chrome place cursor at the end.

comment:2 Changed 6 years ago by Ulrich

Resolution: wontfix
Status: confirmedclosed

comment:3 Changed 6 years ago by Jakub Ś

Resolution: wontfix
Status: closedreopened

If we can't do anything about it please close the issue.

@Ulrich I'm not sure if you have closed the issue because of above comment or because you have deducted that this is a won't fix.

If because of my comment, than I must say sorry for confusing you. I have written that to our senior developers:)

comment:4 Changed 6 years ago by Jakub Ś

Status: reopenedconfirmed

Changed 6 years ago by Garry Yao

Attachment: 7932.patch added

comment:5 Changed 6 years ago by Garry Yao

Keywords: IE added
Owner: set to Garry Yao
Status: confirmedreview
Version: 3.03.1

I've just found a much unobtrusive way to fix #1659, also find the focus grabber stuff is not anymore necessary for other browsers as well, which is introduced back in #5622.

comment:6 Changed 6 years ago by Frederico Caldeira Knabben

Description: modified (diff)

comment:7 Changed 6 years ago by Garry Yao

Summary: Focus does not jump to end when reentering the iframe (in IE8)Focus jump to top when reentering the iframe (in IE8)

comment:8 Changed 6 years ago by Garry Yao

#7326 and #8074 are related.

comment:9 Changed 6 years ago by Sa'ar Zac Elias

Status: reviewreview_failed

IE7 is still affected.

Changed 6 years ago by Garry Yao

Attachment: 7932_2.patch added

Changed 6 years ago by Garry Yao

Attachment: 7932_3.2.patch added

comment:10 Changed 6 years ago by Garry Yao

Status: review_failedreview

New patch takes care of IE6/7, even not having the dragging selection feature in IE8/9/Quirks , it eliminates the selection lost issue.

Due to an Trac error I can't obsolete the patches, 7932_3.2 is the latest.

comment:11 Changed 6 years ago by Lucio Fassio

Garry,

the patches from 7932_2.patch to 7932_3.2.patch cannot be downloaded for an internal error in Trac; can you publish it somewhere else?

Thanks

Changed 6 years ago by Garry Yao

Attachment: 7932_4.patch added

Changed 6 years ago by Garry Yao

Attachment: review.patch added

Changed 6 years ago by Garry Yao

Attachment: 7932_3 added

comment:12 in reply to:  11 Changed 6 years ago by Garry Yao

Replying to lfassio:

Garry,

the patches from 7932_2.patch to 7932_3.2.patch cannot be downloaded for an internal error in Trac; can you publish it somewhere else?

Thanks

It shows me that it's the diff that triggers the Trac error with the patch, 7932_3 is the latest one.

comment:13 Changed 6 years ago by Frederico Caldeira Knabben

Status: reviewreview_failed

I'm not able to apply this patch (malformed). Please try creating it again.

Changed 6 years ago by Garry Yao

Attachment: 7932_3.patch added

comment:14 Changed 6 years ago by Garry Yao

Status: review_failedreview

comment:15 Changed 6 years ago by Frederico Caldeira Knabben

Removing stable code is a very dangerous things. To support it, we need to have tests in place for related tickets: #1659, #5622, #5634 and #5684 (at least). Only after that we'll be able to consider this patch.

comment:16 Changed 6 years ago by Frederico Caldeira Knabben

Keywords: NeedsTest added

comment:17 Changed 6 years ago by Wiktor Walc

Just a note that it might be worth to check #5538 according to comment added by Garry.

comment:18 Changed 6 years ago by Jakub Ś

Oyher related tickets (duplicates) #6359, #6013, #5538.

comment:19 Changed 6 years ago by Jakub Ś

Issues that seem to be fixed by last patch from Garry: #6013, #7932, #5538, #6359, #8658, #8643, #8729, #8074, #7326, #8455

comment:20 Changed 6 years ago by Michael Bu

Cc: gogobu@… added

comment:21 Changed 6 years ago by Frederico Caldeira Knabben

Keywords: NeedsTest removed

comment:22 Changed 6 years ago by Frederico Caldeira Knabben

Status: reviewreview_failed

I've created tests for several related tickets and everything is passing, which indicates that we're almost there.

The test for this ticket is not passing on IE9+Compat, IE8+Compat and IE6 (and probably on IE7 and IE+Quirks). It passes on IE9, IE8 and other browsers.

comment:23 Changed 6 years ago by Ruud Vossen

Cc: ruud@… added

(sorry for the comment. I just want to add myself to cc, but if i leave the comment field blank the content is 'potential spam'. Maybe I do something wrong?)

comment:24 in reply to:  23 Changed 6 years ago by Frederico Caldeira Knabben

Replying to ruud_bmg:

(sorry for the comment. I just want to add myself to cc, but if i leave the comment field blank the content is 'potential spam'. Maybe I do something wrong?)

(That should be ok, sorry... we'll be investigating.)

comment:25 Changed 6 years ago by Matthew Leffler

Cc: mattleff@… added

Adding to CC...

Changed 6 years ago by Garry Yao

Attachment: 7932_5.patch added

comment:26 Changed 6 years ago by Garry Yao

Status: review_failedreview

I didn't mean to address the ticket in IE6/7...while, I agree... we can be a bit better here, so new patch should be enough to align this behavior with IE9.

comment:27 Changed 6 years ago by Frederico Caldeira Knabben

Component: GeneralCore : Selection
Milestone: CKEditor 3.6.3
Status: reviewreview_passed

comment:28 Changed 6 years ago by Garry Yao

Resolution: fixed
Status: review_passedclosed

Fixed with [7374].

comment:29 Changed 6 years ago by Wiktor Walc

~@ruud_bmg - Akismet was rejecting your request to add you to the CC field because it looks like you have set at that time more than one email address in your account settings (the one that is assigned now to the user "gogobu").~

@ruud_bmg - thanks for reporting the problem. Indeed "Akismet says content is spam" when cc-ing as a second person, without posting anything.

Last edited 6 years ago by Wiktor Walc (previous) (diff)

comment:30 Changed 6 years ago by Ruud Vossen

@wwalc: ok, is there anything i can do to fix/prevent that? I don't know the user gogobu, so I don't know how the e-mail address got in my account settings.

comment:31 Changed 6 years ago by Wiktor Walc

@ruud_bmg - sorry for the confusion, I just noticed in logs that you have tried to set the CC field to two email adresses, but in fact it was because gogobu cc-ed himself two weeks earlier. It's a system error, I've marked your earlier changes that failed as "ham", let' s see if it will help.

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