Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#11337 closed Bug (fixed)

[IE10] Listblock tests fail

Reported by: Piotr Jasiun Owned by: Frederico Caldeira Knabben
Priority: Normal Milestone:
Component: General Version:
Keywords: Cc:

Description

http://ckeditor4.t/dt/plugins/listblock/listblock.html

Two first test randomly crash on IE10.

Change History (14)

comment:1 Changed 6 years ago by Piotr Jasiun

Summary: [IE10]listblock test fail[IE10] Listblock tests fail

comment:2 Changed 6 years ago by Marek Lewandowski

Status: newconfirmed

comment:3 Changed 6 years ago by Marek Lewandowski

Owner: set to Marek Lewandowski
Status: confirmedassigned

comment:4 Changed 6 years ago by Marek Lewandowski

Status: assignedreview

The reason for that was a race in CKTester. When CKEDITOR.test() was called, listblock plugin onLoad() method was still not executed, therefore CKEDITOR.ui.listBlock was missing.

You may check it by adding simply:

assert.isInstanceOf( Function, CKEDITOR.ui.listBlock, 'listBlock class is not ready' );

at the begining of first test.

I did not want to introduce any window.setTimeout, so i foreced CKTester to create CKEditor instance before executing test, by putting:

CKTESTER.editor = {};

Solution pushed to t/11337 at tests.

comment:5 Changed 6 years ago by Frederico Caldeira Knabben

While the proposed trick is a workaround that, luckily, makes things work, it looks like we could try to find the source of the problem in CKTester.

I did some research on it and i found that in fact we have problems in the execution order of things in IE, so it makes sense that we have this issue.

I've already coded some things that could help fixing the problem on t/11337b. Still, strangely, a full test run is not working on IE. This means that more research must be done.

comment:6 Changed 6 years ago by Olek Nowodziński

Owner: changed from Marek Lewandowski to Olek Nowodziński
Status: reviewassigned

comment:7 Changed 6 years ago by Olek Nowodziński

Status: assignedreview

Rebased t/11337b on latest master.

I checked ​http://ckeditor4.t/dt/plugins/listblock/listblock.html with t/11337b and there's no more randomness in IE10. I also ran all the tests in IE11, IE10, IE8 and they're green. As for me, the ticket (t/11337b) is R+ but correct me if I missed anything.

comment:8 Changed 6 years ago by Frederico Caldeira Knabben

Status: reviewassigned

Hum... at least with IE10/Win7 I'm having several js errors when running tests on master. It means that we need to stabilize master first and then re-check this ticket.

comment:9 Changed 6 years ago by Olek Nowodziński

Owner: Olek Nowodziński deleted
Status: assignedconfirmed

comment:10 in reply to:  8 Changed 6 years ago by Frederico Caldeira Knabben

Replying to fredck:

Hum... at least with IE10/Win7 I'm having several js errors when running tests on master. It means that we need to stabilize master first and then re-check this ticket.

Ok, CKTester and IE have problems when running tests on iframe. In fact, that's the way it was configured for me. As soon as I set it to "window", all tests run on master without js errors.

Therefore, this ticket is open for investigation again.

comment:11 Changed 6 years ago by Frederico Caldeira Knabben

Owner: set to Frederico Caldeira Knabben
Status: confirmedreview

Ok, everything should be fine. Putting ​t/11337b@tests on review at this point.

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

Status: reviewreview_passed

I checked t/11337b on most of supported browsers. Everything looks fine. However, I wasn't able to reproduce issue described in this ticket on master, so I cannot say that this patch fixes something :). As I don't know CKTester's code, I leave the decision if we need this patch to you.

comment:13 Changed 6 years ago by Frederico Caldeira Knabben

Resolution: fixed
Status: review_passedclosed

Closed with 7e0442c@tests.

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

Milestone: CKEditor 4.3.2

I had to revert this patch because it breaks tests on build version. I reset milestone, because it wasn't really needed - listblock tests started to work without it as I mentioned in comment:12.

Reverted in 746442f@tests.

Fred, if you feel that we should work more on this patch, please open new ticket.

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