Opened 10 years ago
Closed 10 years ago
#13158 closed Bug (fixed)
Error after canceling dialog when creating a widget
| Reported by: | Artur Delura | Owned by: | Artur Delura |
|---|---|---|---|
| Priority: | Normal | Milestone: | CKEditor 4.5.0 |
| Component: | General | Version: | 4.5.0 Beta |
| Keywords: | Cc: |
Description
Browser: Chrome. Version: 4.5.0
- Open: http://tests.ckeditor.dev:1030/tests/plugins/embed/manual/embed.
- Copy some proper link into clipboard.
- Click "embed" button to open dialog.
- Paste link into text input.
- Click "cancel".
- System popup will be shown.
- Click "cancel" in the system popup.
- Click "ok" in the embed popup.
Result: There is an error in console
Cannot read property 'getParent' of null
Change History (13)
comment:1 Changed 10 years ago by
| Status: | new → confirmed |
|---|
comment:2 Changed 10 years ago by
| Milestone: | → CKEditor 4.5.0 |
|---|---|
| Version: | → 4.5.0 (GitHub - major) |
comment:3 Changed 10 years ago by
| Owner: | set to Artur Delura |
|---|---|
| Status: | confirmed → assigned |
comment:4 Changed 10 years ago by
comment:5 Changed 10 years ago by
Checking the "hide" flag first, and preventing from destroy widget will fix the issue.
Good research. The fix sounds correct.
The question is whether event "cancel" should be called in this situation. It doesn't really cancel a dialog. And the second one, if there are more cases like this in our code.
Widgets may be the only case when we do a cleanup on cancel, because they may be the only case when we create an element before the ok button was clicked.
comment:7 Changed 10 years ago by
| Status: | assigned → review |
|---|
comment:8 Changed 10 years ago by
| Status: | review → review_failed |
|---|
The patch and test are good. I miss two small things:
- It would be good to close a dialog used in the test.
- Manual TC (for any widget - can be for the embed one so we can confirm that the issue is really gone).
comment:9 Changed 10 years ago by
| Status: | review_failed → assigned |
|---|
comment:11 Changed 10 years ago by
Please review this issue first, because I need it somehow to work with ticket:13215.
comment:12 Changed 10 years ago by
| Status: | review → review_passed |
|---|---|
| Summary: | Error in embed dialog → Error after canceling dialog when creating a widget |
comment:13 Changed 10 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | review_passed → closed |
Fixed with git:24d1c51.
Note: See
TracTickets for help on using
tickets.
The problem exist in components which listen to "cancel" events on dialog. They simply ignore "hide" flag which is passed in event data.
This property is set in dialog plugin - based on confirm popup which shows to user when there are some changes in dialog. See here.
Here is a listener which lead to error. It destroy widget, although dialog is not closed.
Checking the "hide" flag first, and preventing from destroy widget will fix the issue.
The question is whether event "cancel" should be called in this situation. It doesn't really cancel a dialog. And the second one, if there are more cases like this in our code.
Current changes in branch:t/13158.