Opened 10 years ago
Closed 10 years ago
#12858 closed Task (fixed)
[IE12/Spartan] Bring basic compatibility
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 )
- http://www.winbeta.org/news/microsoft-unveils-developer-channel-internet-explorer-12-ie12
- http://devchannel.modern.ie/#welcome-page
NOTE
If you downloaded Windows 10 before April, then in order 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.
Attachments (2)
Change History (19)
comment:1 Changed 10 years ago by
Status: | new → confirmed |
---|---|
Type: | Bug → Task |
Changed 10 years ago by
Changed 10 years ago by
comment:5 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:6 Changed 10 years ago by
comment:8 Changed 10 years ago by
Owner: | set to Artur Delura |
---|---|
Status: | confirmed → assigned |
comment:9 Changed 10 years ago by
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:11 Changed 10 years ago by
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.
comment:12 Changed 10 years ago by
Status: | review → review_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 10 years ago by
Status: | review_failed → assigned |
---|
comment:14 Changed 10 years ago by
Status: | assigned → review |
---|
Little omission, broken everything. Fixed element.getDocumentPosition to behave like in webkit.
comment:15 Changed 10 years ago by
Description: | modified (diff) |
---|---|
Summary: | [IE12] Check compatibility → [IE12/Spartan] Bring basic compatibility |
comment:16 Changed 10 years ago by
I reinstalled Windows 10 (I wasn't sure if it fully updates right now, so I wanted to be sure that I test the latest version) and I checked the branch:t/12858 again. Now Spartan seems to be more slightly more stable, but it's still far from being ready IMO. Also, it gave me different tests results (compare with comment:12):
tests/core/config/inline Core editor, unit 1 passed / 1 failed / 0 ignored in 183ms tests/core/dom/node Core editor, unit, dom 72 passed / 4 failed / 0 ignored in 287ms tests/core/dom/range/bookmarks Core editor, unit, dom, range 116 passed / 4 failed / 0 ignored in 509ms tests/core/editable/wysiwyg Core editor, unit 5 passed / 1 failed / 0 ignored in 10131ms tests/core/focusmanager/editor Core editor, unit 5 passed / 1 failed / 0 ignored in 721ms tests/core/selection/selection Core editor, unit 24 passed / 6 failed / 1 ignored in 698ms tests/plugins/find/find Plugins editor, unit 2 passed / 1 failed / 0 ignored in 849ms tests/plugins/floatingspace/floatingspace Plugins editor, unit 0 passed / 1 failed / 0 ignored in 107ms tests/plugins/font/font Plugins 2 passed / 10 failed / 0 ignored in 60950ms tests/plugins/image2/link Plugins editor, unit, widget 37 passed / 2 failed / 0 ignored in 7840ms tests/plugins/justify/justify Plugins editor, unit 7 passed / 1 failed / 0 ignored in 1138ms tests/plugins/link/link Plugins editor, unit 15 passed / 1 failed / 0 ignored in 2520ms tests/plugins/sourcearea/source Plugins editor, unit 2 passed / 1 failed / 0 ignored in 126ms tests/plugins/tab/tab Plugins editor, unit 1 passed / 1 failed / 0 ignored in 216ms tests/plugins/xml/xml Plugins editor, unit 3 passed / 1 failed / 0 ignored in 42ms tests/tickets/11500/1 Tickets editor, unit 1 passed / 1 failed / 0 ignored in 184ms
Some tests that were red are now green, but some became red. The most worrisome problems right now are:
- https://twitter.com/reinmarpl/status/585165599371894784 what breaks https://github.com/ckeditor/ckeditor-dev/blob/master/core/editable.js#L1103-L1109,
- that CTRL+A, backspace in classic editor leaves a div instead of paragraph (not that it crashes or something, but it's just ugly and breaks cotent),
- focus is lost in inline editors when opening a color panel.
We will try to address these problems before CKEditor 4.5.0, although, I'm not sure what to do with the first one because it looks like something is broken in the engine.
Other than that and some random "Unspecified errors" and Spartan crashes, the editor works rather well.
comment:17 Changed 10 years ago by
We were curious whether it will be better to recognise Spartan as a next version of IE or as a new browser. I force env.ie to be false and ran some of the tests. Results are not terrible, but not good too. Obviously, far more tests fail, because Spartan is not considered in them. I thought that perhaps manual tests will give better results, but no. Editor works better when Spartan is recognised as IE12. Let's stick to that because this way seems to be far shorter.
comment:18 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:19 Changed 10 years ago by
I fixed this:
https://twitter.com/reinmarpl/status/585165599371894784 what breaks https://github.com/ckeditor/ckeditor-dev/blob/master/core/editable.js#L1103-L1109,
And now the tests results are better:
tests/core/dom/node Core editor, unit, dom 72 passed / 4 failed / 0 ignored in 389ms tests/core/dom/range/bookmarks Core editor, unit, dom, range 116 passed / 4 failed / 0 ignored in 715ms tests/core/focusmanager/editor Core editor, unit 5 passed / 1 failed / 0 ignored in 911ms tests/core/selection/selection Core editor, unit 24 passed / 6 failed / 1 ignored in 794ms tests/plugins/find/find Plugins editor, unit 2 passed / 1 failed / 0 ignored in 771ms tests/plugins/font/font Plugins 2 passed / 10 failed / 0 ignored in 60737ms tests/plugins/image2/link Plugins editor, unit, widget 37 passed / 2 failed / 0 ignored in 9572ms tests/plugins/link/link Plugins editor, unit 15 passed / 1 failed / 0 ignored in 3201ms tests/plugins/tab/tab Plugins editor, unit 1 passed / 1 failed / 0 ignored in 217ms tests/plugins/xml/xml Plugins editor, unit 3 passed / 1 failed / 0 ignored in 38ms
comment:20 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | review → closed |
Fixed on major with git:3263734.
I reported #13142 and #13143 for issues that I mentioned in comment:16.
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.