Opened 4 years ago

Closed 4 years ago

#12398 closed Bug (fixed)

Maximize-Button doesn't work in instances without a title

Reported by: Tobias Hößl Owned by: Artur Delura
Priority: Normal Milestone: CKEditor 4.4.5
Component: General Version: 4.4.4
Keywords: Cc:

Description (last modified by Artur Delura)

When CKEditor is initialized like this...

CKEDITOR.replace(element, { title: '' });

...the Maximize-Button does produces an javascript-error and fails:

Uncaught TypeError: Cannot read property 'setCustomData' of null ckeditor.js:830
Uncaught TypeError: Cannot read property 'setSize' of null ckeditor.js:321

This can be seen here: https://www.hoessl.eu/ckeditor/samples/divreplace.html This is from the standard package, with only one modification: the ", {title: }" was added.

The reason seams to be that no .cke_voice_label-element is created for the instance when the title is empty, but the maximize-plugin thinks the .cke_inner-element is always the second child of its parent, probably in line 128 of plugins/maximize/plugin.js:

var container = editor.container.getChild( 1 );

First bad commit: e7b3238

Change History (9)

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

Keywords: Maximize removed
Milestone: CKEditor 4.4.5
Status: newconfirmed

Thanks. Confirmed and scheduled for the next minor release.

comment:2 Changed 4 years ago by Artur Delura

Owner: set to Artur Delura
Status: confirmedassigned

comment:3 Changed 4 years ago by Artur Delura

Description: modified (diff)

comment:4 Changed 4 years ago by Artur Delura

I wonder whether we can assume that container which we are looking for is always this one which has class cke_inner. If yes, we could use more sophisticated selector that getting element by index.

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

I think we can be sure of that.

What worries me is that I found container.getChild( 1 ) also in themedui.js, so very likely you need to check also that place.

comment:6 in reply to:  5 Changed 4 years ago by Artur Delura

Replying to Reinmar:

I think we can be sure of that.

What worries me is that I found container.getChild( 1 ) also in themedui.js, so very likely you need to check also that place.

I didn't have to check, it just failed when fixed first selector :D

comment:7 Changed 4 years ago by Artur Delura

Status: assignedreview

Changes in branch:t/12398.

comment:8 Changed 4 years ago by Piotrek Koszuliński

The branch was not rebase on latest master. Remember to do that before putting it on review, so you're sure that it's compatible with latest changes at the moment.

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

Resolution: fixed
Status: reviewclosed

Fixed on master with git:9555c193 but I also thought that for safety reasons we should check elements only: git:baa5394.

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