Opened 10 years ago

Closed 10 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 10 years ago.
5221_2.patch (1.2 KB) - added by Garry Yao 10 years ago.
IE Table Error.jpg (31.5 KB) - added by Satya Minnekanti 10 years ago.
5221_3.patch (414 bytes) - added by Garry Yao 10 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 Changed 10 years ago by Frederico Caldeira Knabben

Keywords: Confirmed added
Milestone: CKEditor 3.3

Confirmed with IE8 on trunk also.

comment:2 Changed 10 years ago by Frederico Caldeira Knabben

Version: SVN (CKEditor)3.1

comment:3 Changed 10 years ago by Garry Yao

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

Changed 10 years ago by Garry Yao

Attachment: 5221.patch added

comment:4 Changed 10 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 10 years ago by Frederico Caldeira Knabben

Description: modified (diff)

comment:6 Changed 10 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 10 years ago by Garry Yao

Attachment: 5221_2.patch added

comment:7 Changed 10 years ago by Garry Yao

Keywords: Review? added; Review- removed

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

comment:8 Changed 10 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 10 years ago by Garry Yao

Resolution: fixed
Status: assignedclosed

Fixed with [5263] and [5264].

comment:10 Changed 10 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 10 years ago by Satya Minnekanti

Attachment: IE Table Error.jpg added

Changed 10 years ago by Garry Yao

Attachment: 5221_3.patch added

comment:11 Changed 10 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 10 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 10 years ago by Garry Yao

Resolution: fixed
Status: reopenedclosed

Fixed with [5327].

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