Ticket #6170 (closed Bug: fixed)

Opened 4 years ago

Last modified 4 years ago

Incorrect CSS classes applied to rich combo menu panel.

Reported by: JoeK Owned by: Saare
Priority: Normal Milestone: CKEditor 3.4.2
Component: UI : Toolbar Version: 3.4 Beta
Keywords: IBM Cc: damo satya

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

6170.patch (1.2 KB) - added by Saare 4 years ago.
6170_2.patch (1.2 KB) - added by Saare 4 years ago.
6170_3.patch (1.4 KB) - added by Saare 4 years ago.
6170_4.patch (455 bytes) - added by garry.yao 4 years ago.
6170_5.patch (2.2 KB) - added by Saare 4 years ago.

Change History

comment:1 Changed 4 years ago by tobiasz.cudnik

  • Status changed from new to confirmed
  • Version changed from 3.4 Beta to 3.5 (SVN - 3.5.x)

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

comment:2 Changed 4 years ago by fredck

  • Version changed from 3.5 (SVN - 3.5.x) to 3.4 Beta

comment:3 Changed 4 years ago by fredck

  • Milestone set to CKEditor 3.4.1

comment:4 Changed 4 years ago by Saare

  • Status changed from confirmed to assigned
  • Owner set to Saare

Changed 4 years ago by Saare

comment:5 Changed 4 years ago by Saare

  • Status changed from assigned to review

comment:6 Changed 4 years ago by tobiasz.cudnik

  • Status changed from review to review_passed

comment:7 Changed 4 years ago by tobiasz.cudnik

  • Status changed from review_passed to review_failed

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

Changed 4 years ago by Saare

comment:8 Changed 4 years ago by Saare

  • Status changed from review_failed to review

comment:9 Changed 4 years ago by garry.yao

  • Status changed from review to 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 4 years ago by Saare

comment:10 Changed 4 years ago by Saare

  • Status changed from review_failed to review

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

Changed 4 years ago by garry.yao

comment:11 Changed 4 years ago by garry.yao

  • Status changed from review to review_failed

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

comment:12 Changed 4 years ago by Saare

  • Status changed from review_failed to 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 4 years ago by Saare

comment:13 Changed 4 years ago by Saare

The new patch covers all panels.

comment:14 Changed 4 years ago by garry.yao

  • Status changed from review to review_passed

comment:15 Changed 4 years ago by Saare

  • Status changed from review_passed to closed
  • Resolution set to fixed

Fixed with [5909].

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