Ticket #5221 (closed Bug: fixed)

Opened 5 years ago

Last modified 4 years ago

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

Reported by: JoeK Owned by: garry.yao
Priority: Normal Milestone: CKEditor 3.3
Component: General Version: SVN (CKEditor) - OLD
Keywords: IBM Confirmed Review+ Cc: damo

Description (last modified by fredck) (diff)

  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

5221.patch (630 bytes) - added by garry.yao 5 years ago.
5221_2.patch (1.2 KB) - added by garry.yao 5 years ago.
IE Table Error.jpg (31.5 KB) - added by satya 4 years ago.
5221_3.patch (414 bytes) - added by garry.yao 4 years ago.

Change History

comment:1 Changed 5 years ago by fredck

  • Keywords Confirmed added
  • Milestone set to CKEditor 3.3

Confirmed with IE8 on trunk also.

comment:2 Changed 5 years ago by fredck

  • Version changed from SVN (CKEditor) to 3.1

comment:3 Changed 5 years ago by garry.yao

  • Owner set to garry.yao
  • Status changed from new to assigned
  • Version changed from 3.1 to SVN (CKEditor)

Changed 5 years ago by garry.yao

comment:4 Changed 5 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 5 years ago by fredck

  • Description modified (diff)

comment:6 Changed 5 years ago by fredck

  • 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 5 years ago by garry.yao

comment:7 Changed 5 years ago by garry.yao

  • Keywords Review? added; Review- removed

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

comment:8 Changed 4 years ago by fredck

  • 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 4 years ago by garry.yao

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

Fixed with [5263] and [5264].

comment:10 Changed 4 years ago by satya

  • Status changed from closed to reopened
  • Resolution fixed deleted
  • Summary changed from [IE7 3.2.x branch] Have to click the Table dialog's OK button twice when there is no content. to [[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 4 years ago by satya

Changed 4 years ago by garry.yao

comment:11 Changed 4 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 4 years ago by fredck

  • Keywords Review+ added; Review? removed

Please replace the change with the following when committing:

this.moveToPosition( dummySpan, CKEDITOR.POSITION_BEFORE_START );

comment:13 Changed 4 years ago by garry.yao

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

Fixed with [5327].

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