Opened 8 years ago

Closed 8 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: Alfonso Martínez de Lizarrondo
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 Alfonso Martínez de Lizarrondo 8 years ago.
Proposed patch
4682_2.patch (2.0 KB) - added by Garry Yao 8 years ago.
4682_3.patch (1.3 KB) - added by Alfonso Martínez de Lizarrondo 8 years ago.
Revised patch

Download all attachments as: .zip

Change History (15)

comment:1 Changed 8 years ago by Garry Yao

Keywords: Confirmed added
Milestone: CKEditor 3.2
Version: 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 8 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.2CKEditor 3.3

comment:3 Changed 8 years ago by Kingsquare

Cc: ckeditor@… added

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

comment:4 Changed 8 years ago by Sijmen Ruwhof

Cc: sruwhof@… added
Milestone: CKEditor 3.3CKEditor 3.2
Summary: Content of div with width is not editable in IE7Content 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 8 years ago by Alfonso Martínez de Lizarrondo

Milestone: CKEditor 3.2CKEditor 3.3

Changed 8 years ago by Alfonso Martínez de Lizarrondo

Attachment: 4682.patch added

Proposed patch

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

Keywords: Review? added
Owner: set to Alfonso Martínez de Lizarrondo
Status: newassigned

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 8 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 8 years ago by Garry Yao

Attachment: 4682_2.patch added

comment:8 Changed 8 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 8 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 8 years ago by Alfonso Martínez de Lizarrondo

Attachment: 4682_3.patch added

Revised patch

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

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 8 years ago by Garry Yao

Keywords: Review+ added; Review? removed

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

Resolution: fixed
Status: assignedclosed

Fixed with [5214]

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