Opened 5 years ago

Closed 5 years ago

#12978 closed Bug (fixed)

About uploading causes error in notification aggregator

Reported by: Piotr Jasiun Owned by: Artur Delura
Priority: Normal Milestone: CKEditor 4.5.0
Component: General Version: 4.5.0 Beta
Keywords: Cc:

Description

After merging notification aggregator with upload plugin I met this issue in the upload manual test:

http://tests.ckeditor.dev:1030/tests/plugins/uploadwidget/manual/image

When I remove image during uploading:

plugin.js:261 Uncaught TypeError: Cannot read property 'update' of null

Change History (9)

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

Status: newconfirmed

I've got this error too but I couldn't reproduce it. It must be some timing issue.

comment:2 in reply to:  1 Changed 5 years ago by Piotr Jasiun

Replying to Reinmar:

I've got this error too but I couldn't reproduce it. It must be some timing issue.

The issue is because XHR mock call "onload" even if upload was aborted. This should be fixed.

comment:3 Changed 5 years ago by Artur Delura

Aborting uploading image is not case in this manual test. We sholdn't care if something fail when tester did an unexpected action. We hardcoded success here which means that we don't except aborting.

I assume that we don't have any abroting test in our code, when we write the first one, then we should parametrize this but now we ain't gonna need this.

comment:4 Changed 5 years ago by Piotr Jasiun

The success (uploaded: 1) is on load. Everything you need to do is to set flag on about https://github.com/cksource/ckeditor-dev/blob/dc2501f4c34a3c2489abcb9e19d5ff72b167ff86/tests/plugins/uploadwidget/manual/_helpers/xhr.js#L73 and do not call onprogress and onload if upload was aborted. I do not understand why XHR mock should behave differently than normal XHR. Also this is not that edge case if user remove uploading image or do 'undo'. We slow down uploading for this manual test exactly do let him do this.

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

Milestone: CKEditor 4.5.0 BetaCKEditor 4.5.0

comment:6 Changed 5 years ago by Artur Delura

Owner: set to Artur Delura
Status: confirmedassigned

As far as I can see the description changes since I commented this issue so now this test is more general.

comment:7 Changed 5 years ago by Artur Delura

need to do is to set flag

Don't know which flag you mean, but I think that clearing interval is enough. I won't put it on review until someone can confirm this. Changes in branch:t/12978.

comment:8 Changed 5 years ago by Artur Delura

Status: assignedreview

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

Resolution: fixed
Status: reviewclosed
Version: 4.5.0 Beta

I couldn't reproduce any error on major, but I reproduced different issues. For example - when dropping many files and then cancelling upload of one of them, progress was reaching >100%. Now it's fixed.

Merged to major with git:c666d44.

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