Opened 5 years ago

Last modified 5 years ago

#12858 closed Task

[IE12/Spartan] Bring basic compatibility — at Version 15

Reported by: Piotrek Koszuliński Owned by: Artur Delura
Priority: Must have (possibly next milestone) Milestone: CKEditor 4.5.0 Beta
Component: General Version:
Keywords: Cc:

Description (last modified by Piotrek Koszuliński)

NOTE

To test IE12 vel Spartan you need to go to about:flags and click "enable" in the "Enable Experimental Web Platform Features" section. Now IE12 is discovered as Chrome (you can verify this in dev tools - there should be "edge", not "11" in the right top corner of the dev tools panel.

You may also need to disable Compatibility View, because by default in the intranet IE starts in this mode.

NOTE2

As for now, Spartan isn't really usable so it will not be possible to bring a full compatibility with it. Dev tools are often unstable (what makes development a nightmare), weird stuff happens and new Windows updates bring rather drastic changes. Therefore, we'll focus now on the biggest changes that we are rather sure that we need to make on our side to have at least some basic level of compatibility. Further changes will be made in separate tickets, most likely after CKEditor 4.5.0.

Change History (14)

comment:1 Changed 5 years ago by Jakub Ś

Status: newconfirmed
Type: BugTask

Changed 5 years ago by Piotrek Koszuliński

Attachment: ie8.png added

Changed 5 years ago by Piotrek Koszuliński

Attachment: ie12.png added

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

Description: modified (diff)

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

More info. When IE12 is recognised as Chrome it works very well. I had around 30 red tests after running 1500. However, when I forced discovering it as IE12 I started having errors preventing editor initialisation. A lot more work needs to be put into this.

comment:8 Changed 5 years ago by Artur Delura

Owner: set to Artur Delura
Status: confirmedassigned

comment:9 Changed 5 years ago by Artur Delura

Main problem in IE12 was caused because in current code features were implemented using browser detection instead of feature detection. It's quite straightforward that in new IE some old and unstandarised implementations were removed.

So far I worked out core code (all tests pass) and magicline and wysiwygarea plugin. Current changes in branch:t/12858.

comment:10 Changed 5 years ago by Artur Delura

Status: assignedreview

So far so good.

comment:11 Changed 5 years ago by Artur Delura

So far so good.

In IE12 we got compareDocumentPosition so function CKEditor.dom.element.prototype.contains use it if available. Same for getComputedStyle and CKEditor.dom.element.prototype.getComputedStyle. Improved CKEditor.dom.element.prototype.getTabIndexto be browser independent and based on this what tabIndex` property gives us.

Simplified getStyle internal function because it uses improved CKEditor.dom.element.prototype.getComputedStyle function.

runtimeStyle property has been removed in IE12, so don't rely on it.


Lessons learned:

  • Future detection over browser detection.
  • If something is not a standard but browser specific implementation we should take into consideration that in the future version it might be removed:
    if ( ie ) {
        console.log( element.runtimeStyle );
    }

In future IE it won't work.


Things to be done:

  • tests/plugins/font/font
  • tests/plugins/image/image : test update image (attributes)
  • tests/plugins/widget/widgetselection
  • tests/plugins/xml/xml
  • tests/plugins/clipboard/pastedialog : test paste dialog focus
  • tests/plugins/find/find
  • tests/plugins/magicline/magicline
  • tests/core/dom/element/element
  • tests/core/focusmanager/editor

This tests fails still.

Last edited 5 years ago by Artur Delura (previous) (diff)

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

Status: reviewreview_failed

I pushed branch:t/12858 with one additional commit.

My reds right now:

tests/core/dom/element/element Core editor, unit, dom   115 passed / 1 failed / 1 ignored in 1173ms   
tests/core/focusmanager/editor Core editor, unit   5 passed / 1 failed / 0 ignored in 955ms   
tests/plugins/clipboard/pastedialog Plugins editor, unit, dialog    3 passed / 1 failed / 0 ignored in 829ms   
tests/plugins/find/find Plugins editor, unit    1 passed / 2 failed / 0 ignored in 849ms   
tests/plugins/font/font Plugins     2 passed / 10 failed / 0 ignored in 60987ms   
tests/plugins/image2/editing Plugins editor, unit, widget    5 passed / 1 failed / 0 ignored in 2693ms   
tests/plugins/justify/justify Plugins editor, unit   7 passed / 1 failed / 0 ignored in 1524ms   
tests/plugins/magicline/magicline Plugins editor, unit, magicline   27 passed / 1 failed / 9 ignored in 3504ms   
tests/plugins/widget/widgetselection Plugins editor, unit, widgetcore    7 passed / 3 failed / 0 ignored in 31188ms   
tests/plugins/xml/xml Plugins editor, unit   3 passed / 1 failed / 0 ignored in 49ms  

From all these tests only one is important - that in the http://tests.ckeditor.dev:1030/tests/core/dom/element/element. The method that fails is getDocumentPosition() and it's perfectly visible when doing manual tests because all menus and floating toolbars are mispositioned. This is must have to claim that the support level is ok to close this ticket.

Besides this, the changes that you done, Artur, broke IE8 totally. You must be careful when doing feature detection.

comment:13 Changed 5 years ago by Artur Delura

Status: review_failedassigned

comment:14 Changed 5 years ago by Artur Delura

Status: assignedreview

Little omission, broken everything. Fixed element.getDocumentPosition to behave like in webkit.

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

Description: modified (diff)
Summary: [IE12] Check compatibility[IE12/Spartan] Bring basic compatibility
Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy