Opened 14 years ago

Closed 14 years ago

#4611 closed Bug (fixed)

Bug when insert SELECT in a form

Reported by: Sylvain 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 Sylvain 14 years ago.
4611.patch (1.3 KB) - added by Garry Yao 14 years ago.
4611_2.patch (712 bytes) - added by Garry Yao 14 years ago.

Download all attachments as: .zip

Change History (14)

Changed 14 years ago by Sylvain

Attachment: bugCKEDITOR.jpg added

comment:1 Changed 14 years ago by Frederico Caldeira Knabben

Keywords: Confirmed IE added
Milestone: CKEditor 3.xCKEditor 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 14 years ago by Garry Yao

Component: GeneralCore : Styles
Milestone: CKEditor 3.1CKEditor 3.2
Version: 3.0.1SVN (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 14 years ago by Garry Yao

Attachment: 4611.patch added

comment:3 Changed 14 years ago by Garry Yao

Keywords: Review? added
Owner: set to Garry Yao
Status: newassigned

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 14 years ago by Frederico Caldeira Knabben

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 14 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 14 years ago by Frederico Caldeira Knabben

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

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

Attachment: 4611_2.patch added

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

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 14 years ago by Garry Yao

Resolution: fixed
Status: assignedclosed

Fixed with [5102].

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy