Opened 7 years ago

Closed 6 years ago

#9597 closed Bug (invalid)

Config.height accepts percentage value

Reported by: Piotrek Koszuliński Owned by: Olek Nowodziński
Priority: Normal Milestone:
Component: General Version: 4.0
Keywords: Cc:

Description

It shouldn't accept value like '50%', but it does and tries to do something. The result is broken - on Chrome I have huge editor, when on FF it's tiny.

Change History (7)

comment:1 Changed 7 years ago by Jakub Ś

Status: newconfirmed

In v3 behaviour has been much more consistent. There has been tiny editor across all browsers :).
Value 50% has been assigned to td with class cke_contents.

<td role="presentation" style="height: 50%;" class="cke_contents" id="cke_contents_editor1">...</td>

NOTE: when you have used min/max toolbar button this value is being changed to 50px making editor content area even smaller.


In v4 this value is being assigned to div with class cke_contents. <div role="presentation" class="cke_contents cke_reset" id="cke_1_contents" style="height: 50%;">...</div> Most likely because of different HTML element (DIV not TD) and way how browsers handle this by default, results are so weird.


Perhaps a solution is to simply replace editor height with default px value if % is detected or computing the value and assigning it in px.

comment:2 Changed 6 years ago by Garry Yao

The computed percentage size doesn't make sense as well since it doesn't scale, it's simple to just skip the config in this case.

comment:3 Changed 6 years ago by Olek Nowodziński

Owner: set to Olek Nowodziński
Status: confirmedassigned

comment:4 Changed 6 years ago by Olek Nowodziński

Status: assignedreview

Created t/9597@cksource with the solution: assign CKEDITOR.config.height if editor.config.height is invalid. Still I'm not sure if empty string (something similar) is better in this case.

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

Status: reviewreview_failed

But this still won't work if I set:

CKEDITOR.config.height = '50%';

I think that default height should be moved from config to code.

comment:6 in reply to:  5 Changed 6 years ago by Olek Nowodziński

Replying to Reinmar:

But this still won't work if I set:

CKEDITOR.config.height = '50%';

I think that default height should be moved from config to code.

Nice catch. Let's wait for Fred to take a decision.

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

Milestone: CKEditor 4.0.1
Resolution: invalid
Status: review_failedclosed

We don't have to care about handling incorrect values. Documentation says that percentage values are not accepted.

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