#13867 closed Bug (fixed)
CKEditor fails to load on IE8/9 when a polyfill for classList is included
| Reported by: | sam | Owned by: | Marek Lewandowski |
|---|---|---|---|
| Priority: | Normal | Milestone: | CKEditor 4.5.5 |
| Component: | General | Version: | 4.5.4 |
| Keywords: | Drupal | Cc: | theodore@…, wim.leers@… |
Description
Steps to reproduce
- load a classList polyfill onto the page with polyfill from https://developer.mozilla.org/en/docs/Web/API/Element/classList
- load ckeditor onto the same page after the polyfill
Expected result
Everything should work nice and fine.
Actual result
There will be errors on the page about this.$.classList is undefined for elements within the text-editor iframe $(".cke_wysiwyg_frame .cke_reset").
All the features will not work except the very basic textbox.
Other details (browser, OS, CKEditor version, installed
My environment was IE8 on windows 7, but I believe this can happen on ie9 as well. The ckeditor version was 4.5.4.
My obseravtion is that the elements within the iframe need to also detect if classList is available, instead of doing the detection just once during page load.
I traced to this commit. https://github.com/ckeditor/ckeditor-dev/commit/0836608
Change History (16)
comment:1 Changed 9 years ago by
| Cc: | theodore@… added |
|---|---|
| Keywords: | Drupal added |
comment:3 Changed 9 years ago by
| Cc: | wim.leers@… added |
|---|
Reported against Drupal 8 too: https://www.drupal.org/node/2607362
comment:4 Changed 9 years ago by
| Milestone: | → CKEditor 4.5.5 |
|---|---|
| Status: | new → confirmed |
Let's try to solve it in 4.5.5.
comment:5 Changed 9 years ago by
| Summary: | Ckeditor on IE8/9 fail to load when a polyfill for classList is included → CKEditor fails to load on IE8/9 when a polyfill for classList is included |
|---|
RE: milestone=4.5.5: perfect, that matches what I wrote/expected in https://www.drupal.org/node/2321583#comment-10527008.
(Also cleaned up the title.)
comment:6 Changed 9 years ago by
| Owner: | set to Marek Lewandowski |
|---|---|
| Status: | confirmed → assigned |
comment:7 Changed 9 years ago by
| Status: | assigned → review |
|---|
comment:8 Changed 9 years ago by
| Status: | review → review_failed |
|---|
The fix is not working correctly. Note that IE8 throws 'HTMLElement' is undefined as TypeError. Maybe we should check not only the type of exception, but also the message? Or just rewrite the check to first check if HTMLElement is defined before trying to access its properties.
comment:9 Changed 9 years ago by
| Status: | review_failed → review |
|---|
Correct! Additionally descriptors added to IE8 by the polyfill returns typeof unknown which made this approach more complicated.
Instead here is an alternative, and way simpler approach.
comment:11 Changed 9 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | review_passed → closed |
Fixed with git:a8460e1271 (merged to master).
comment:12 Changed 9 years ago by
(!!!) First of all, an external library in CKEditor project requires an entry in LICENSE.md.
Secondly, I hardly find it necessary because a mockup of the classList library would be enough to prove that https://github.com/ckeditor/ckeditor-dev/commit/a8460e1271#diff-19bd701718906bfcb56a997c506cc452R158 check works correctly.
comment:15 Changed 9 years ago by
Replaced original snippet with a stubbed object on branch branch:t/13867c. Also I was able to create unit test to keep us safe from regressions. @a.nowodzinski take a look on it to see if you can see any room for improvement. If not we'll merge it to master.
I can confirm that. I'm having difficulties on my IE8 module for Drupal 8, since we add a classList polyfill by default.
Can reproduce on XP-IE8.