Ticket #4611 (closed Bug: fixed)

Opened 10 months ago

Last modified 7 months ago

Bug when insert SELECT in a form

Reported by: tsylvai Owned by: garry.yao
Priority: Normal Milestone: CKEditor 3.2
Component: Core : Styles Version: SVN (CKEditor) - OLD
Keywords: Confirmed IE Review+ Cc:

Description

When i insert a HTML/Form/Select in page i have an javascript error Message "nodeValue.length is null or isn't an object."

This message appear after insert the select (Click ok on the dialog Select Popup) and the popup dialog not close

I have the same bug in your site with the demo page "Editor with all features"

IE 6 & 7

Attachments

bugCKEDITOR.jpg Download (107.8 KB) - added by tsylvai 10 months ago.
4611.patch Download (1.3 KB) - added by garry.yao 7 months ago.
4611_2.patch Download (0.7 KB) - added by garry.yao 7 months ago.

Change History

Changed 10 months ago by tsylvai

Changed 10 months ago by fredck

  • keywords Confirmed IE added
  • milestone changed from CKEditor 3.x to CKEditor 3.1

Confirmed with IE8. It doesn't happen if the dialog fields are left empty. You must fill them, adding some entries at the options list.

Changed 10 months ago by garry.yao

  • version changed from 3.0.1 to SVN (CKEditor)
  • component changed from General to Core : Styles
  • milestone changed from CKEditor 3.1 to CKEditor 3.2

It's a IE selection bug, only occurs in the following situation:

<form>^<select><option value="value">text</option></select></form>

Where the attempt to reveal a range from it will break.
We could defer it, as usually <select> is will be following other inline elements or text, in which case the bug will not be triggered.

Changed 7 months ago by garry.yao

Changed 7 months ago by garry.yao

  • keywords Review? added
  • owner set to garry.yao
  • status changed from new to assigned

The culprit in is that IE report text option value inside <select> as ordinary text, which breaks the range measurement logics:

<form>[<select><option value="value">text</option></select>]</form>
alert( ieRange.htmlText );  //<select><option value="value">text</option></select>
alert( ieRange.text );  //text

Changed 7 months ago by fredck

  • keywords Review- added; Review? removed

The fix looks good, but we should reduce the usage cases of it as it may impact on the performance.

As <select> is the only known case of it, we should have a "<select" check there to understand whether to use the new counting logic.

Changed 7 months ago by garry.yao

After a re-check into the problem, I found IE7's selection around a <select> is far more buggy than we discovered, the range measurement is almost broken around it, e.g.

Environment

IE7, IE8 Compat

Reproducing Procedures

  1. Open any sample page, load the editor with the following source and selection:
     <form>text<select><option>value</option></select>te^xt</form>
    
  2. Press 'Bold' button;
  • Expected Result: The bold style is opened in place.
  • Actual Result: The bold style is opened inside the text node before the select element.

Considering the uncertainty we're facing here and the fact that IE8 already fixed the bug, I'm proposing of deferring this ticket.

Changed 7 months ago by fredck

We should at least avoid the js error for now, even if the selection will not be properly done.

Changed 7 months ago by garry.yao

The error comes directly from the range measurement codes, without correcting the measurement, it's hard to guarantee the fix.

Changed 7 months ago by garry.yao

Expose the exact IE bug behind is when 'oElement' in the following method is an <SELECT>, the range will move to the end of the current paragraph instead of surrounding the element.

//<p>text<select></select>text</p>
object.moveToElementText(oElement); // Move to <select>
// Expected : <p>text[<select></select>]text</p>
//Actual: <p>text<select></select>text</p>^

Changed 7 months ago by garry.yao

Changed 7 months ago by garry.yao

  • keywords Review? added; Review- removed

Considering we could do few thing to fix this bug I'm proposing a safe-guard patch that doesn't guarantee correct range measurement but indented to be js error proof.

Changed 7 months ago by fredck

  • keywords Review+ added; Review? removed

Ok, this is not a definitive solution but it minimizes the problem. At least we don't have js errors happening.

Please add a comment to the try block, pointing to this ticket before committing.

Changed 7 months ago by garry.yao

  • status changed from assigned to closed
  • resolution set to fixed

Fixed with [5102].

Note: See TracTickets for help on using tickets.