Opened 7 years ago

Closed 6 years ago

#4611 closed Bug (fixed)

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 (3)

bugCKEDITOR.jpg (107.8 KB) - added by tsylvai 7 years ago.
4611.patch (1.3 KB) - added by garry.yao 7 years ago.
4611_2.patch (712 bytes) - added by garry.yao 6 years ago.

Download all attachments as: .zip

Change History (14)

Changed 7 years ago by tsylvai

comment:1 Changed 7 years 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.

comment:2 Changed 7 years ago by garry.yao

  • Component changed from General to Core : Styles
  • Milestone changed from CKEditor 3.1 to CKEditor 3.2
  • Version changed from 3.0.1 to SVN (CKEditor)

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

comment:3 Changed 7 years 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

comment:4 Changed 6 years 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.

comment:5 Changed 6 years 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.

comment:6 Changed 6 years ago by fredck

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

comment:7 Changed 6 years ago by garry.yao

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

comment:8 Changed 6 years 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 6 years ago by garry.yao

comment:9 Changed 6 years 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.

comment:10 Changed 6 years 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.

comment:11 Changed 6 years ago by garry.yao

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

Fixed with [5102].

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