Opened 10 years ago
Closed 10 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 follow-up: 2 Changed 10 years ago by
| Status: | new → confirmed |
|---|
comment:2 Changed 10 years ago by
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 10 years ago by
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 10 years ago by
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 10 years ago by
| Milestone: | CKEditor 4.5.0 Beta → CKEditor 4.5.0 |
|---|
comment:6 Changed 10 years ago by
| Owner: | set to Artur Delura |
|---|---|
| Status: | confirmed → assigned |
As far as I can see the description changes since I commented this issue so now this test is more general.
comment:7 Changed 10 years ago by
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 10 years ago by
| Status: | assigned → review |
|---|
comment:9 Changed 10 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | review → closed |
| 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.
I've got this error too but I couldn't reproduce it. It must be some timing issue.