Opened 6 years ago

Closed 6 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)

test-master.zip (2.3 KB) - added by Wiktor Walc 6 years ago.
11490.html (2.1 KB) - added by Piotrek Koszuliński 6 years ago.

Download all attachments as: .zip

Change History (21)

Changed 6 years ago by Wiktor Walc

Attachment: test-master.zip added

comment:1 Changed 6 years ago by Wiktor Walc

Status: newconfirmed

comment:2 Changed 6 years ago by Marek Lewandowski

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 elementPath method, checking current mode.

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

cc

comment:4 Changed 6 years ago by Wiktor Walc

Milestone: CKEditor 4.3.3

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

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 6 years ago by Marek Lewandowski

Owner: set to Marek Lewandowski
Status: confirmedassigned

comment:7 Changed 6 years ago by Marek Lewandowski

Pushed solution to t/11490 at dev and t/11490 at tests.

comment:8 Changed 6 years ago by Marek Lewandowski

Status: assignedreview

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

Status: reviewreview_failed

Selection should not be retrieved if startNode is passed.

comment:10 Changed 6 years ago by Marek Lewandowski

Pushed to t/11490.

comment:11 Changed 6 years ago by Marek Lewandowski

Status: review_failedreview

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

Status: reviewreview_failed

The tests is in a wrong place - it's a test for elementsPath method, not for sourcearea.

comment:13 Changed 6 years ago by Marek Lewandowski

Status: review_failedreview

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 6 years ago by Piotrek Koszuliński

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 6 years ago by Marek Lewandowski

Status: reviewassigned

comment:16 Changed 6 years ago by Marek Lewandowski

Status: assignedreview

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 6 years ago by Piotrek Koszuliński

Attachment: 11490.html added

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

Status: reviewreview_passed

Force pushed dev and tests after rebasing and improving both branches.

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

Summary: menubutton does not work in source modeMenubutton does not work in source mode

comment:19 Changed 6 years ago by Marek Lewandowski

Resolution: fixed
Status: review_passedclosed

fixed with git:c96cd55 at dev master, and e846582 at tests master

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