Opened 8 years ago

Closed 8 years ago

#3494 closed Bug (fixed)

Context menu is too narrow in some translations

Reported by: Artur Formella Owned by: Alfonso Martínez de Lizarrondo
Priority: Normal Milestone: CKEditor 3.2
Component: UI : Floating Panel Version: SVN (CKEditor) - OLD
Keywords: Confirmed Review+ Cc:

Description

Context menu is too narrow in some translations. Please see attached image.

Attachments (3)

3494.jpg (39.7 KB) - added by Artur Formella 8 years ago.
3494.patch (2.1 KB) - added by Alfonso Martínez de Lizarrondo 8 years ago.
Partial patch
3494_2.patch (3.9 KB) - added by Alfonso Martínez de Lizarrondo 8 years ago.
Context menu patch

Download all attachments as: .zip

Change History (14)

Changed 8 years ago by Artur Formella

Attachment: 3494.jpg added

comment:1 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Confirmed added
Milestone: CKEditor 3.0CKEditor 3.1

We should have auto-width for the menus, just like V2.

comment:2 Changed 8 years ago by Frederico Caldeira Knabben

#4034 has been marked as DUP.

comment:3 Changed 8 years ago by Frederico Caldeira Knabben

#4568 has been marked as DUP.

comment:4 Changed 8 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.1CKEditor 3.2

comment:5 Changed 8 years ago by Alfonso Martínez de Lizarrondo

Owner: set to Alfonso Martínez de Lizarrondo
Status: newassigned

Changed 8 years ago by Alfonso Martínez de Lizarrondo

Attachment: 3494.patch added

Partial patch

comment:6 Changed 8 years ago by Alfonso Martínez de Lizarrondo

Keywords: Review? added

I would like to know if the current approach for the patch is right.

Making it work for Firefox, Opera and Safari is quite straight forward, but IE is quite different as expected: if the panel is loaded with the alpha opacity filter applied, then the elements don't overflow, so it's not possible to get their real scrollWidth. In order to workaround that, I have moved that attributes in the kama skin to a new rule with a class that it's set only after the width has been computed.

I haven't tried to test thoroughly the patch, I just want to know if the coding looks OK or if it can be done in a better way. In the skin I've disabled also the _width rule for IE6, I think that the _overflow can be removed also, I'll test that approach for the final patch unless someone states a reason why they might be required.

comment:7 Changed 8 years ago by Garry Yao

Component: GeneralUI : Floating Panel
Keywords: Review- added; Review? removed
Version: SVN (CKEditor)

The patch works well for me with the following limitations:

  1. Doesn't work for other panels (combobox);
  2. Doesn't work in Safari;

comment:8 Changed 8 years ago by Alfonso Martínez de Lizarrondo

I was focusing just on the context menu, as I thought that other panels are discussed in #4559 Should I work also on that? It could be a matter of moving this new code outside of the if ( block.autoSize ) check but I don't want to do it if that's not the expected behavior.

A last minute change broke compatibility with Safari, but as I said I was more worried about the structure of the code (changing the skin to include a new class in the frame). If that's ok, I'll work to review that it works properly in all browsers.

Thanks for the comments.

Changed 8 years ago by Alfonso Martínez de Lizarrondo

Attachment: 3494_2.patch added

Context menu patch

comment:9 Changed 8 years ago by Alfonso Martínez de Lizarrondo

Keywords: Review? added; Review- removed

The patch covers only the context menu. I've tried to make it work for other floating panels, but I can't find what's messing there with the widths in IE, I'm not able to make it work, so I propose to fix the context menu for the moment and focus later in the rest of panels.

I've tested it quickly in RTL as I don't know which language might have long translations for the context menu.

comment:10 Changed 8 years ago by Garry Yao

Keywords: Review+ added; Review? removed

The patch works well for context menu.

comment:11 Changed 8 years ago by Alfonso Martínez de Lizarrondo

Resolution: fixed
Status: assignedclosed

Fixed with [5047]

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