Opened 5 years ago

Closed 5 years ago

#10612 closed Bug (fixed)

IE11 Compatibility

Reported by: Frederico Caldeira Knabben Owned by: Piotrek Koszuliński
Priority: Normal Milestone: CKEditor 4.3
Component: General Version:
Keywords: IE11 Cc: frietsch@…, mattleff@…

Description (last modified by Frederico Caldeira Knabben)

This is the main ticket to check IE11 compatibility. It is also an umbrella ticket for each specific issue related to it.

Status: assigned (1 match)

Ticket Summary Keywords Owner Type Priority Milestone
#13575 IE11: Typing in Korean with Enter Mode BR moves cursor to previous line. IE11 Support Szymon Cofalik Bug Normal

Status: confirmed (22 matches)

Ticket Summary Keywords Owner Type Priority Milestone
#10616 IE11 numbers lists from zero IN COMPATIBILITY MODE IE11 Bug Normal
#10792 Error in IE10 With Compatibility View IE10 IE11 Support Bug Normal
#11060 [IE11+] Selection is not placed right above the cursor when clicking below body IE11 Bug Normal
#11234 IE 11 doesn't like links with display:inline-block IE11 Bug Normal
#11324 checkDirty() return true, but should return false Firefox IE11 Bug Normal
#11346 Formatting lost when applied to highlighted section, cursor placed at end of line, then the same formatting option toggled again Firefox IE11 Bug Normal
#11429 [IE11] Can't place space at the beginning of text input IE11 Bug Normal
#11778 IE11: The xml object loaded with Ajax plugin fails to find children IE10 IE11 Bug Normal
#11840 [IE11] Editable fixDom causes selection to lose direction IE11 Bug Normal
#11878 [IE11]: Applying different numbers to list doesn’t work. IE11 Bug Normal
#12453 CKEditor in IE11 doesn't handle large tables. IE11 Support Bug Normal
#13041 menus not working in showmodaldialog IE10 IE11 Bug Normal
#13043 Inline styles are not maintained in IE11 when using Korean. IE11 Bug Normal
#13061 IE11: Pasting images and shapes from word duplicates some and omits others IE11 Bug Normal
#13509 [IE11] It is not possible to type in Japanese in right aligned table IE11 Support Can't Fix Bug Normal
#13630 [IE11] Focus issues when the editor gets loaded IBM, IE11 Bug Normal
#13775 Typing into selected contents of span tag creates font tag IE11 Blink Webkit Bug Normal
#14525 Japanese charactor disappeared while typing IE11 Bug Normal
#14704 IE11: Japanese Kanji character breaks when you repeatedly typing Japanese IBM IE11 Bug Normal
#14767 Browser becomes unresponsive at the input of the multi-byte character IE11 Bug Normal
#16681 Pasting List from Word in IE wraps list utems in paragraphs. IE11 Bug Normal
#16718 [IE11][CF] Styles applied to the wrong node when CF used exactly on the same position for the second time. IE11, Copyformatting Bug Normal

Status: pending (1 match)

Ticket Summary Keywords Owner Type Priority Milestone
#14733 setting Italic fails for Japanese character, in Japanese locale IBM IE11 Bug Normal

Status: review (2 matches)

Ticket Summary Keywords Owner Type Priority Milestone
#11663 Input Method Editor(IMEs) didn't work properly in CKEditor with the AutoGrow plugin in IE11 IBM IE11 Support Szymon Cofalik Bug Normal
#13911 [IE11] Console errors during context menu test execution IE11 kkrzton Bug Normal

Change History (32)

comment:1 Changed 5 years ago by Piotrek Koszuliński

cc

comment:2 Changed 5 years ago by Frederico Caldeira Knabben

First findings:

  • CKEDITOR.env.ie == false && CKEDITOR.env.gecko == true... so CKEditor sees it as Gecko.
  • If we force env.ie to true, no editor appears any more... and no error as well :/

It should be wrong to have env.gecko == true, because I assume that IE doesn't need Gecko hacks. At the same time it looks like it doesn't need many of the previous IE hacks as well.

If we are able to have the editor created with env.ie == true and it accepts well the IE hacks, that could be already a first step towards compatibility. I think this should be our first task.

comment:3 Changed 5 years ago by frietsch

Cc: frietsch@… added

comment:4 Changed 5 years ago by Frederico Caldeira Knabben

Description: modified (diff)
Status: newconfirmed

comment:5 Changed 5 years ago by Matthew Leffler

Cc: mattleff@… added

CCing...

comment:6 Changed 5 years ago by Alfonso Martínez de Lizarrondo

Instead of checking how to set env.ie == true, I think that you should and a env.newIE == true (testing for Gecko and Trident in the UA) so that the old IE hacks don't apply by default and instead it follows a path more similar to the rest of the browsers, adding hacks only when needed instead of trying to figure out which old hacks are no longer needed.

After all, MS has worked hard to be treated like the rest of the browsers so forcing it to work like old IE isn't polite to them. http://www.nczonline.net/blog/2013/07/02/internet-explorer-11-dont-call-me-ie/

comment:7 in reply to:  6 Changed 5 years ago by Frederico Caldeira Knabben

Replying to alfonsoml:

I agree that we should go on these lines, of adding a new env property exclusive for IE11+... internally, we've been talking about env.giecko :D but something like env.newIE, as you proposed, is certainly more appropriate.

comment:8 Changed 5 years ago by Piotrek Koszuliński

We may want to fix also #10679 when working on this ticket. Especially that CC statements are not supported in Windows Store Apps (http://msdn.microsoft.com/en-us/library/ie/121hztk3(v=vs.94).aspx) and may be dropped at any time in future IE releases.

comment:9 Changed 5 years ago by Wiktor Walc

Just to not forget: Windows 8.1 will be generally available on October, 18th, so it'd be good to detect and fix all the major issues till then.

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

comment:10 Changed 5 years ago by Piotr Jasiun

Owner: set to Piotr Jasiun
Status: confirmedassigned

comment:11 Changed 5 years ago by Piotr Jasiun

And what do you think about env.geckoIE or env.ieGecko? I really don't like "newIE" because what if IE 15 will be based on webKit? We will call it newNewIE? And in 2 years IE11 will be nothing new and nobody will understand why we call it "newIE".

comment:12 Changed 5 years ago by Piotrek Koszuliński

That's why we should have ie2 - short, and can be further incremented. No one remember real IE2 version, so it's less likely that someone will be confused (what's possible with proposed by you ie11).

comment:13 Changed 5 years ago by Piotr Jasiun

On stand-up meeting we decided to use if( env.ie && env.vesion <= 10 )

comment:14 Changed 5 years ago by Piotr Jasiun

I've changed all env.ie to env.ie && env.vesion <= 10. There are still more than 50 not passing test and plenty of bugs and errors, but editor on IE 11 works... somehow. Anyway we have not enough time before beta to make it perfect.

comment:15 Changed 5 years ago by Piotr Jasiun

Please, someone make a review of these changes.

comment:16 Changed 5 years ago by Piotr Jasiun

Status: assignedreview

comment:17 Changed 5 years ago by Frederico Caldeira Knabben

Status: reviewreview_failed

I've pushed a few commits to the branch.

Still we don't have anything good, with these test results on IE9: 17 failed /1419 passed /02m 24s 617ms - 100%.

So, it needs work.

comment:18 Changed 5 years ago by Frederico Caldeira Knabben

The funny part of it: the "dev built" version of ckeditor.js is now 3KB bigger after this change, but it is 25B smaller when gzipped :)

comment:19 Changed 5 years ago by Piotrek Koszuliński

Still we don't have anything good, with these test results on IE9: 17 failed /1419 passed /02m 24s 617ms - 100%.

Well... this is a bad news that something stopped working on IE9 on which theoretically we don't change anything. Having changes in 300 places it will be very hard to revert those which changed code semantics for IE9.

comment:20 Changed 5 years ago by Piotr Jasiun

It does not change anything. If you switch to major commit which is base for this branch you will get the same results. Maybe test branch and dev branch were created in different moments on different states of major, but it not the change what is the reason of this not passing tests. For sure not so many. The problem is, that it may be not so easy to rebase this ticket. And it will be not easy at all to merge it with widget branch.

comment:21 Changed 5 years ago by Piotrek Koszuliński

So if these branches are desynchronized, then synchronizing it again is a first thing that should be done. It's not possible to work on them if you don't know what and why fails.

comment:22 Changed 5 years ago by Piotrek Koszuliński

Regarding t/9764, I think that there won't be a lot of problems. But t/9764 should land on major first, because it is >300 commits long. Then rebasing t/10612 on major won't be so tricky, because it will be just one longer commit and most likely not so much conflicts. t/9764 doesn't touch a lot of browser specific code - most of the changes are architectural.

Last edited 5 years ago by Piotrek Koszuliński (previous) (diff)

comment:23 Changed 5 years ago by Frederico Caldeira Knabben

Let's wait for the 4.3 beta to get closed, so we can work on this before doing any other big thing starts.

comment:24 Changed 5 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 4.3 BetaCKEditor 4.3

comment:25 Changed 5 years ago by Piotrek Koszuliński

Owner: changed from Piotr Jasiun to Piotrek Koszuliński
Status: review_failedassigned

comment:26 Changed 5 years ago by Piotrek Koszuliński

Status: assignedreview

I pushed t/10612b on dev and tests with much shorter patch than in t/10612. I changed only one CKEDITOR.env.ie check in resourceManager, because onreadystatechange is not supported in IE11. This, and few insignificant changes in tests gave me 100% green.

Unfortunately, this proved once more that automated tests will never replace manual ones. I found that IE11 really differs from its predecessors. For example it started using bogus <brs> as filler nodes what means that we'll need to review very carefully all places which deal with bogus <brs> and include IE11+ there. See more in #10907.

I also noticed few issues (pretty bad ones in case of inline editors) with focus - e.g. opening a dropdown list causes editor#blur, but they are not constantly reproducible, so I'm not reporting them yet.

Anyway, I'm putting this ticket (t/10612b branches) on review, because I'd like to merge those basic, but very important changes to major as soon as possible to make testing easier (it's not possible now on major/master) and to avoid being flooded by issues report for IE11 recognised as Firefox (I already closed some).

comment:27 Changed 5 years ago by Piotrek Koszuliński

PS. All other issues which are still reproducible on t/10612b, unless critical, should be reported as separate tickets (like #10907 and #10906), so we'll avoid having one huge branch making dozens of tricky and confusing changes.

comment:28 Changed 5 years ago by Piotrek Koszuliński

PPS. I want to keep this ticket opened for now, because it is an umbrella ticket for all issues, but as we started some basics fixes in t/10612b and they are a base for other patches, I want to have t/10612b merged into major ASAP.

comment:29 Changed 5 years ago by Frederico Caldeira Knabben

Status: reviewreview_passed

Nice step forward for compatibility IE11. Let's keep opening tickets for the remaining issues.

comment:30 Changed 5 years ago by Piotrek Koszuliński

I merged t/10612b into major at git:8eb1a58 and 153a77d on tests. Let's keep this ticket opened for now to keep track of all other issues.

comment:31 Changed 5 years ago by Piotrek Koszuliński

Status: review_passedassigned

comment:32 Changed 5 years ago by Frederico Caldeira Knabben

Resolution: fixed
Status: assignedclosed

We should have reached the first milestone here. We'll followup on IE11 compatibility through separate tickets now.

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