Opened 16 years ago

Closed 16 years ago

#6170 closed Bug (fixed)

Incorrect CSS classes applied to rich combo menu panel.

Reported by: Joe Kavanagh Owned by: Sa'ar Zac Elias
Priority: Normal Milestone: CKEditor 3.4.2
Component: UI : Toolbar Version: 3.4 Beta
Keywords: IBM Cc: Damian Satya Minnekanti

Description

A single div is used to display the style, format, font and font size menu panels.

When the menu is being dynamically created the corresponding menu's CSS class is added.

For example, when creating the Font Size menu the CSS class cke_fontSize_panel is added. When creating the Formatting Styles menu the CSS class cke_styles_panel is added.

No CSS class is ever removed.

If all four richcombo menus are used the div incorrectly contains the CSS classes cke_font_panel, cke_fontSize_panel, cke_format_panel and cke_styles_panel. This can lead to the incorrect styles being applied.

Attachments (5)

6170.patch​ (1.2 KB) - added by Sa'ar Zac Elias 16 years ago.
6170_2.patch​ (1.2 KB) - added by Sa'ar Zac Elias 16 years ago.
6170_3.patch​ (1.4 KB) - added by Sa'ar Zac Elias 16 years ago.
6170_4.patch​ (455 bytes) - added by Garry Yao 16 years ago.
6170_5.patch​ (2.2 KB) - added by Sa'ar Zac Elias 16 years ago.

Download all attachments as: .zip

Change History (20)

comment:1 Changed 16 years ago by Tobiasz Cudnik

Status: new → confirmed
Version: 3.4 Beta → 3.5 (SVN - 3.5.x)

Key to reproduce this is to open a next combo without closing a previous one.

comment:2 Changed 16 years ago by Frederico Caldeira Knabben

Version: 3.5 (SVN - 3.5.x) → 3.4 Beta

comment:3 Changed 16 years ago by Frederico Caldeira Knabben

Milestone: → CKEditor 3.4.1

comment:4 Changed 16 years ago by Sa'ar Zac Elias

Owner: set to Sa'ar Zac Elias
Status: confirmed → assigned

Changed 16 years ago by Sa'ar Zac Elias

Attachment: 6170.patch​ added

comment:5 Changed 16 years ago by Sa'ar Zac Elias

Status: assigned → review

comment:6 Changed 16 years ago by Tobiasz Cudnik

Status: review → review_passed

comment:7 Changed 16 years ago by Tobiasz Cudnik

Status: review_passed → review_failed

"class" isn't an allowed identifier, it causes IE8 to not parse the source.

Changed 16 years ago by Sa'ar Zac Elias

Attachment: 6170_2.patch​ added

comment:8 Changed 16 years ago by Sa'ar Zac Elias

Status: review_failed → review

comment:9 Changed 16 years ago by Garry Yao

Status: review → review_failed

A much DRY approach would be trying to trigger the previous panel's hide handler directly so everything get cleaned up, not only the class name in this case.

Changed 16 years ago by Sa'ar Zac Elias

Attachment: 6170_3.patch​ added

comment:10 Changed 16 years ago by Sa'ar Zac Elias

Status: review_failed → review

Added the preventOnClose parameter so the solution won't trigger the onClose method unnecessarily.

Changed 16 years ago by Garry Yao

Attachment: 6170_4.patch​ added

comment:11 Changed 16 years ago by Garry Yao

Status: review → review_failed

Let's make it simpler, note that font color panels are also affected.

comment:12 Changed 16 years ago by Sa'ar Zac Elias

Status: review_failed → review

Garry and me agreed that we should preserve the current behaviour of the onClose event. I'm opting for a second look before expanding the patch also to the font color and bg color panels.

Changed 16 years ago by Sa'ar Zac Elias

Attachment: 6170_5.patch​ added

comment:13 Changed 16 years ago by Sa'ar Zac Elias

The new patch covers all panels.

comment:14 Changed 16 years ago by Garry Yao

Status: review → review_passed

comment:15 Changed 16 years ago by Sa'ar Zac Elias

Resolution: → fixed
Status: review_passed → closed

Fixed with [5909].

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy