Opened 10 years ago

Closed 10 years ago

#3363 closed Bug (fixed)

Font and Size controls are not working correctly

Reported by: Senthil Owned by: Martin Kou
Priority: Normal Milestone: CKEditor 3.0
Component: General Version:
Keywords: Oracle Confirmed IE Review+ Cc: Senthil

Description

Replication steps:

  1. Select a paragraph of text from the editor area.
  2. Use the Size control to make the text larger (select 16 in the control). Notice that size 16 indicated in the control.
  3. Use the Size control and select size 16 again. The text size becomes smaller again. Notice that no size is indicated in the control.

Same thing is happening with Font control also.

Attachments (2)

3363.patch (1.1 KB) - added by Martin Kou 10 years ago.
3363_2.patch (1.1 KB) - added by Martin Kou 10 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 10 years ago by Garry Yao

This is related to #3351.

comment:2 Changed 10 years ago by Garry Yao

Keywords: Pending added

The Font/Size combo is designed to be toggle-able, not sure if something else is not working as expected besides the marking problem at #3351.

comment:3 Changed 10 years ago by Martin Kou

Resolution: invalid
Status: newclosed

The font/size toggle is actually a feature rather than a bug - I made a mistake when explaining it to Senthil yesterday because I didn't see the toggle highlight.

So closing this bug as invalid.

comment:4 Changed 10 years ago by Martin Kou

Keywords: Confirmed IE added; Pending removed

Just talked with Senthil today, the combos are still not working correctly even when taking into account the toggle behavior. To reproduce the bug:

  1. Open replacebyclass.html in IE.
  2. Select "some sample text".
  3. Change the font to "Comic Sans MS".
  4. Change the font to "Courier New"
  5. Now we end up with both Comic Sans MS and Courier New mixed up in the selected text.

A similar bug also happens with the font size combo.

comment:5 Changed 10 years ago by Martin Kou

Resolution: invalid
Status: closedreopened

comment:6 Changed 10 years ago by Martin Kou

Owner: set to Martin Kou
Status: reopenednew

Changed 10 years ago by Martin Kou

Attachment: 3363.patch added

comment:7 Changed 10 years ago by Martin Kou

Keywords: Review? added

Ok.. took me a whole day to figure this out. It's caused by a rather hideous browser bug in IE.

Turns out the (sibling.$.offsetWidth > 0) check we use in CKEDITOR.dom.range::enlarge() doesn't really filter out the bookmark nodes we're adding to the document. IE is reporting an offsetWidth of 3 even though the bookmark nodes have display: none in their CSS styles. This caused the enlarge() function to ignore the existing <span> style tags when applying font and font sizes.

The solution is to add a check for display: none in the element's computed style.

comment:8 in reply to:  7 Changed 10 years ago by Garry Yao

Replying to martinkou: This' great to see yet another bookmark ignorance problem be dug out, but I wonder if we can use:

sibling.$.offsetWidth > 0 && !sibling.getAttribute( '_fck_bookmark' )

Instead of

sibling.$.offsetWidth > 0 && sibling.getComputedStyle( 'display' ) != 'none'

I'm thinking of the getComputedStyle slowness when invoking on large amounts.

Changed 10 years ago by Martin Kou

Attachment: 3363_2.patch added

comment:9 Changed 10 years ago by Martin Kou

Yes, that's a good idea. It shouldn't happen often that we have display: none elements.

comment:10 Changed 10 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

comment:11 Changed 10 years ago by Martin Kou

Resolution: fixed
Status: newclosed

Fixed with [3408].

Click here for more info about our SVN system.

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