Opened 8 years ago

Closed 8 years ago

#5221 closed Bug (fixed)

[[IE]] Have to click the Table dialog's OK button twice when there is no content.

Reported by: Joe Kavanagh Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.3
Component: General Version: SVN (CKEditor) - OLD
Keywords: IBM Confirmed Review+ Cc: Damian

Description (last modified by Frederico Caldeira Knabben)

  1. Open the Ajax sample. Notice that the elements path is empty.
  2. Click the Table toolbar button.
  3. Click the Table dialog's OK button.

Observe the javascript error: 'type' is null or not an object.

This does not occur on the nightly 3.2 demo as the content area gets focus before the dialog appears. "body p" appears in the elements path.

The error is occurring in core/dom/walker.js guardRTL function.

I believe the error is occurring because the node to replace is null. The wysiwyg plugin's onInsertElement function thinks it is in an empty p element and attempts to replace it with the table element. See the if statement following the below comment in the onInsertFunction function:

If we're in an empty block which indicate a new paragraph, simply replace it with the inserting block.(#3664)

Attachments (4)

5221.patch (630 bytes) - added by Garry Yao 8 years ago.
5221_2.patch (1.2 KB) - added by Garry Yao 8 years ago.
IE Table Error.jpg (31.5 KB) - added by Satya Minnekanti 8 years ago.
5221_3.patch (414 bytes) - added by Garry Yao 8 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Confirmed added
Milestone: CKEditor 3.3

Confirmed with IE8 on trunk also.

comment:2 Changed 8 years ago by Frederico Caldeira Knabben

Version: SVN (CKEditor)3.1

comment:3 Changed 8 years ago by Garry Yao

Owner: set to Garry Yao
Status: newassigned
Version: 3.1SVN (CKEditor)

Changed 8 years ago by Garry Yao

Attachment: 5221.patch added

comment:4 Changed 8 years ago by Garry Yao

Keywords: Review? added

Defer the table element insertion to avoid a race-condition with the auto paragraph feature which happens to mangle the range.

comment:5 Changed 8 years ago by Frederico Caldeira Knabben

Description: modified (diff)

comment:6 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

I was wondering if it's possible to fix the race condition in the auto paragraph code. I this way our API "just works". I'm reviewing- it for investigation.

Changed 8 years ago by Garry Yao

Attachment: 5221_2.patch added

comment:7 Changed 8 years ago by Garry Yao

Keywords: Review? added; Review- removed

You're right, we do have better option here.

comment:8 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

Let's just have a clearer implementation of restoreSelection when committing it:

	function restoreSelection( selection )
	{
		if ( selection.isLocked )
		{
			selection.unlock();
			setTimeout( function() { selection.lock(); }, 0 );
		}
	}

comment:9 Changed 8 years ago by Garry Yao

Resolution: fixed
Status: assignedclosed

Fixed with [5263] and [5264].

comment:10 Changed 8 years ago by Satya Minnekanti

Resolution: fixed
Status: closedreopened
Summary: [IE7 3.2.x branch] Have to click the Table dialog's OK button twice when there is no content.[[IE]] Have to click the Table dialog's OK button twice when there is no content.

This is still re producible on the nightly build.you may not get it the first time you open the editor.Try opening and closing the Editor a couple of times.

To Reproduce the defect:

  1. Open Ajax Sample.
  1. Click on Create Editor button.
  1. Click on Table Icon to get Table Properties dialog.
  1. Click on OK button to Insert the Table.

We are getting an Error and i have attached the Screen shot for the Error.

We have to click OK button second time to insert the Table.

Changed 8 years ago by Satya Minnekanti

Attachment: IE Table Error.jpg added

Changed 8 years ago by Garry Yao

Attachment: 5221_3.patch added

comment:11 Changed 8 years ago by Garry Yao

Keywords: Review? added; Review+ removed

It's not a regression, instead it's unbelievable that we still have logic hole in range::select method.

comment:12 Changed 8 years ago by Frederico Caldeira Knabben

Keywords: Review+ added; Review? removed

Please replace the change with the following when committing:

this.moveToPosition( dummySpan, CKEDITOR.POSITION_BEFORE_START );

comment:13 Changed 8 years ago by Garry Yao

Resolution: fixed
Status: reopenedclosed

Fixed with [5327].

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