Opened 7 years ago

Closed 6 years ago

#4682 closed Bug (fixed)

Content of any tag with width or height property is not editable in IE7 and IE8

Reported by: Dekel Owned by: alfonsoml
Priority: Normal Milestone: CKEditor 3.3
Component: General Version: SVN (CKEditor) - OLD
Keywords: Confirmed Review+ Cc: ckeditor@…, sruwhof@…

Description

When creating a div with width (in source mode), the content of the div is not editable in the WYSIWYG mode. Code for example:

<div style="width: 300px">
	<p>
		1234<br />
		5678</p>
</div>

Also checked on the Nightly build (current ver: 4483) Firefox doesn't seem to have this bug.

Attachments (3)

4682.patch (1.6 KB) - added by alfonsoml 7 years ago.
Proposed patch
4682_2.patch (2.0 KB) - added by garry.yao 6 years ago.
4682_3.patch (1.3 KB) - added by alfonsoml 6 years ago.
Revised patch

Download all attachments as: .zip

Change History (15)

comment:1 Changed 7 years ago by garry.yao

  • Keywords Confirmed added
  • Milestone set to CKEditor 3.2
  • Version set to SVN (CKEditor)

Back in v2 we could start editing the content of such element by double clicking on it, we should have the same result for v3.

comment:2 Changed 7 years ago by fredck

  • Milestone changed from CKEditor 3.2 to CKEditor 3.3

comment:3 Changed 7 years ago by Kingsquare

  • Cc ckeditor@… added

Same problem exists in IE 8, in both 'full page' as in 'snippet' mode...

comment:4 Changed 7 years ago by sruwhof

  • Cc sruwhof@… added
  • Milestone changed from CKEditor 3.3 to CKEditor 3.2
  • Summary changed from Content of div with width is not editable in IE7 to Content of any tag with width or height property is not editable in IE7 and IE8

I have experienced similair problems in IE 8.0: text inside the tag can't be selected for editing purposes. Other browsers (Firefox, Safari, Opera and Chrome) don't have the problem.

The bug is more generic:

  • it seems that tags other than <div> also have the problem (I've tried <h1> and <span>).
  • not only the property 'width' causes the problem, but also the 'height' property (I've tried <div>, <h1>, <span>).

-- Sijmen Ruwhof

comment:5 Changed 7 years ago by alfonsoml

  • Milestone changed from CKEditor 3.2 to CKEditor 3.3

Changed 7 years ago by alfonsoml

Proposed patch

comment:6 follow-up: Changed 7 years ago by alfonsoml

  • Keywords Review? added
  • Owner set to alfonsoml
  • Status changed from new to assigned

I think that I've found a solution to the problem by being careful in the onFocusIn listener. After that, when trying to get out of the edited element raised a Stack Overflow like #5114 so I've added a check verifying if the element has layout besides being a table element (maybe that check could be changed now, but it's better to be safe and don't show a blinking cursor instead of getting an overflow)

With this patch the behavior is the same than FCKeditor, so elements with layout are editable after clicking and waiting a little. This can help to fix #4910 as now it will be possible to position the hidden div (at least I hope so)

comment:7 Changed 6 years ago by garry.yao

  • Keywords Review- added; Review? removed

The selection restore logics introduced in v3 was the criminal, while the patch doesn't work in the following case:

Environment

IE7, IE8 Compat

Reproducing Procedures

  1. Load the document with the following contents:
    <p style="float:right">layouted paragraph</p>
    
  2. Double left click to put cursor inside the layouted paragraph.
  3. Select the word "paragraph" and press 'Bold' button.
  • Expected Result: The bold style is applied to the text.
  • Actual Result: Stack overflow error is thrown.

Changed 6 years ago by garry.yao

comment:8 Changed 6 years ago by garry.yao

I've made necessary guards to the 'beforedeactivate' event handler in symmetry with the 'focusin' handler seems resolving this problem.

comment:9 in reply to: ↑ 6 Changed 6 years ago by garry.yao

Replying to alfonsoml:

This can help to fix #4910...

Definitely! You can now give a review on the new patch at that ticket.

Changed 6 years ago by alfonsoml

Revised patch

comment:10 Changed 6 years ago by alfonsoml

  • Keywords Review? added; Review- removed

I've left out in the patch the part of #4716 because that fix creates #5203, and it seems that just taking care of the $.toElement is enough to avoid the stack overflow errors in all the cases (including the first adjustment for tables in [5100])

But as I said, that part should be handled at its ticket.

comment:11 Changed 6 years ago by garry.yao

  • Keywords Review+ added; Review? removed

comment:12 Changed 6 years ago by alfonsoml

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

Fixed with [5214]

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