Opened 11 years ago
Closed 11 years ago
#11490 closed Bug (fixed)
Menubutton does not work in source mode
| Reported by: | Wiktor Walc | Owned by: | Marek Lewandowski |
|---|---|---|---|
| Priority: | Normal | Milestone: | CKEditor 4.3.3 |
| Component: | General | Version: | 4.3 |
| Keywords: | Cc: |
Description
After this commit:
https://github.com/ckeditor/ckeditor-dev/commit/ddf9074ad04d18d5ba6a8dcc1a9162936e455bc2
menubutton stopped working in source mode. After pressing the menubutton, the dropdown menu does not open when the editor is in source mode.
Firefox reports: TypeError: this.getSelection(...) is null
Chrome reports: Uncaught TypeError: Cannot call method 'getStartElement' of null
The attached plugin can be used to reproduce this issue.
Attachments (2)
Change History (21)
Changed 11 years ago by
| Attachment: | test-master.zip added |
|---|
comment:1 Changed 11 years ago by
| Status: | new → confirmed |
|---|
comment:2 Changed 11 years ago by
comment:4 Changed 11 years ago by
| Milestone: | → CKEditor 4.3.3 |
|---|
comment:5 Changed 11 years ago by
Editor#elementPath() should return null if getSelection() returned null, because getSelection() returns null in other modes than wysiwyg. Then null values have to be handled correctly in menubutton code.
comment:6 Changed 11 years ago by
| Owner: | set to Marek Lewandowski |
|---|---|
| Status: | confirmed → assigned |
comment:8 Changed 11 years ago by
| Status: | assigned → review |
|---|
comment:9 Changed 11 years ago by
| Status: | review → review_failed |
|---|
Selection should not be retrieved if startNode is passed.
comment:11 Changed 11 years ago by
| Status: | review_failed → review |
|---|
comment:12 Changed 11 years ago by
| Status: | review → review_failed |
|---|
The tests is in a wrong place - it's a test for elementsPath method, not for sourcearea.
comment:13 Changed 11 years ago by
| Status: | review_failed → review |
|---|
But it's elementpath function of core editor object, it's not a part of elementspath plugin. So what's the other proposition? should we make core code aware of sourcearea plugin?
comment:14 Changed 11 years ago by
Editor#elementPath is an editor's method so its tests belongs to editor's API tests. And it's not aware of sourcearea plugin - it's aware of editor.mode property. You can test it by overriding editor.mode value or by using sourcearea for that to have perfectly realistic test.
Additionally, it would be good to have two menubutton integration tests - one for each mode.
comment:15 Changed 11 years ago by
| Status: | review → assigned |
|---|
comment:16 Changed 11 years ago by
| Status: | assigned → review |
|---|
Tests repushed, as of integration test with both sourcearea and wysiwygarea i've so far wasted 2 hrs, with buggy effect so i'm dropping it. It's a candidate for separate ticket.
Changed 11 years ago by
| Attachment: | 11490.html added |
|---|
comment:17 Changed 11 years ago by
| Status: | review → review_passed |
|---|
Force pushed dev and tests after rebasing and improving both branches.
comment:18 Changed 11 years ago by
| Summary: | menubutton does not work in source mode → Menubutton does not work in source mode |
|---|
comment:19 Changed 11 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | review_passed → closed |
fixed with git:c96cd55 at dev master, and e846582 at tests master
Reason for that issue is that menu plugin calls editor#elementPath() method twice, in onShow() and show() methods, each of these calls causes now mentioned exception when we're in sourceview mode. Fix may be simply implemented in
elementPathmethod, checking current mode.