Opened 14 years ago
Last modified 12 years ago
#7561 review_failed Bug
IE 'editor.getSelection()' is null or not an object
Reported by: | Michael Camden | Owned by: | Garry Yao |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | Core : Selection | Version: | 3.5.2 |
Keywords: | IE8 HasPatch IBM | Cc: | mcamden@…, satya_minnekanti@…, tkrah@…, joel.peltonen@… |
Description
This error is sometimes thrown in IE. It happens onClick and I've tracked it down to the following bit of code.
plugins/selection/plugin.js
Ln #215: editor.getSelection().getRanges()[ 0 ].select();
According to the documentation, and my observations, editor.getSelection() can return null. This statement should be wrapped in a conditional block to avoid calling .getRanges on a null object.
if((selection = editor.getSelection()) != null) { selection.getRanges()[ 0 ].select(); }
Attachments (1)
Change History (19)
comment:1 Changed 14 years ago by
Keywords: | IE8 HasPatch added; IE 8 null or not an object removed |
---|---|
Status: | new → pending |
comment:2 Changed 14 years ago by
Thus far it has been pretty random. It seems to most often occur after setData has been called, and I click on the editable section of the editor (not creating a selection).
comment:3 Changed 14 years ago by
I'm also getting an editor.getSelection is null error on _source/plugins/wysiwygarea/plugin.js line 831 caused by: range = editor.getSelection().getRanges()[ 0 ];
I'm getting the error only when i add a smiley in the editor. If i delete just text everything works fine. The statements: if ( body.getHtml().match( /<p> <\/p>$/i )
&& range.startContainer.equals( body ) )
are true. But when i add a smiley in the editor (<img>) i get the error.
I added some alerts and found that after deleting everything with a smiley on event 'selectionchange' the range.startContainer is not 'body', container is 'p' I'm not sure what's happening since i haven't checked deep the code for that.
comment:4 Changed 14 years ago by
So If I understand you correctly, you add smiley to sample text and when you delete all you get an error. I did that and still didn't get any errors. I have tried to select text with mouse and using keyboard but none of this actions resulted in success (error).
To know what is wrong and how to fix it, first we have to find out what is causing the error. That is why it would be really nice if we could make this issue reproducible.
Are you using some special configuration settings or something other which is non-standard? Could you paste your config.js and perhaps the code form html page that you are using to create editor?
comment:5 Changed 14 years ago by
aorduno is referring to our own smilie inserts, not the smilie code included with CKEditor. Our inserts are done by calls to editor.insertHtml() so nothing exciting going on there. We can not reproduce this on all IE7 installations. The gist of it that it occurs most often if we take an editor with text, remove that text, then insert the smilie using editor.insertHtml. The offending code is in the
editor.config.enterMode != CKEDITOR.ENTER_P
&& domDocument.on( 'selectionchange', function()
block on line 819 of the wysiwygarea plugin.
We occasionally have failures on:
body.getFirst() editor.getSelection() editor.getSelection().getRanges()
Our suggestion is to check these values for null before continuing in this block of code:
domDocument.getBody() editor.getSelection() editor.getSelection().getRanges()
We've added code to perform these checks and no longer experience the infrequent javascript errors from trying to use null values.
Going beyond this to determine why these values are null is up to you.
comment:6 Changed 14 years ago by
Keywords: | HasPatch removed |
---|
This ticket got a bit confusing. I was wondering if anyone could attach a sample TC page to confirm the issue, as well as a patch or changed code with the fixes, so we can understand better where to touch the code. Thanks!
comment:7 Changed 14 years ago by
We have this as our fix:
Edit _source/plugins/wysiwygarea/plugin.js
around line 822 find:
var body = domDocument.getBody(), range = editor.getSelection().getRanges()[ 0 ];
Replace With:
if (domDocument.getBody() && editor.getSelection() && editor.getSelection().getRanges()) { var body = domDocument.getBody(); range = editor.getSelection().getRanges()[ 0 ]; } else { return; }
around line 838 find:
range = editor.getSelection().getRanges()[ 0 ]; if (!range.startContainer.equals ( 'body' ) ) { body.getFirst().remove( 1 ); range.moveToElementEditEnd( body ); range.select( 1 ); }
Replace With:
if (editor.getSelection() && editor.getSelection().getRanges()) { range = editor.getSelection().getRanges()[ 0 ]; if (!range.startContainer.equals ( 'body' ) ) { body.getFirst().remove( 1 ); range.moveToElementEditEnd( body ); range.select( 1 ); } }
comment:8 Changed 14 years ago by
Keywords: | HasPatch added |
---|
comment:9 Changed 14 years ago by
We've started to experience this now on IE. I can only reproduce the problem in a production build i.e. I have not been able to find a simple stand-alone test case to reproduce this issue.
It is consistently reproducible. The issue can be reproduced by creating an instance of the editor, destroying it, then creating a new one. After this, without focusing in the content area, invoke a dialog (e.g. Find, Table etc). This always causes the problem, on all dialogs.
We get the exact same error that is tracked down to document.getSelection() returning null from the function editor.getSelection().
I see that there are a few places in the code that assume the result from getSelection() is always valid, yet the API documentation says it may return null.
comment:10 Changed 14 years ago by
Cc: | satya_minnekanti@… added |
---|---|
Keywords: | IBM added |
comment:12 Changed 13 years ago by
I'm able to reliable reproduce this with the following code in IE:
var editor = CKEDITOR.instances['editor1']; editor.setData(<p id="p">p</p>'); var element = editor1.document.getById( 'p' ); editor.getSelection().selectElement( element );
Changed 13 years ago by
Attachment: | 7561.patch added |
---|
comment:13 Changed 13 years ago by
Owner: | set to Garry Yao |
---|---|
Status: | pending → review |
I would propose to force editor focus upon selection retrieve, considering it's a frequent trapped pitfall for developers.
comment:14 Changed 13 years ago by
comment:15 Changed 13 years ago by
Below is an extremely simple page for seeing this same issue (getSelection returning null). I consistently get a javascript error using ckeditor 3.6.2 in IE 9.0.8112.16421 on Windows 7 when in "Compatibility View". To get the error, I just change the text in the editor and then click the "Update Element" button. The error occurs after both of the alerts and is "Unable to get value of the property 'getRanges': object is null or undefined" Line: 99, Char: 2198.
<html> <head> <script type="text/javascript" src="/ckeditor/ckeditor.js"></script> </head> <body> <form name="testForm" action="" method="post" onsubmit="return false"> <textarea id="test" name="test">abc</textarea> <input type="button" name="testButton" value="Update Element" onclick="alert('before'); CKEDITOR.instances.test.updateElement(); alert(this.form.test.value);"> </form> <script>CKEDITOR.replace("test", {enterMode: CKEDITOR.ENTER_BR});</script> </body> </html>
comment:16 Changed 13 years ago by
Cc: | tkrah@… added |
---|
Add me to CC list (maybe this comment does prevent kismet from telling me my request for CC is spam ...).
comment:17 Changed 13 years ago by
Status: | review → review_failed |
---|
- comment:9: I'm not able to reproduce this with the Ajax sample.
- comment:12: I can reproduce this on trunk with IE9+Compat.
- comment:15: I can reproduce this on trunk revision 7275 (CKEditor 3.6.2) but not on HEAD.
So, the only justification for this fix would be one case related to third party code requiremenets. Only if one of the participants could give more information about the above cases.
In any case, one of the test created for it is not passing on IE8+Compat. This needs to be checked before having the patch accepted.
This clearly looks like a piece of code to improve.
I know you have written "sometimes" but perhaps you are able to tell us at least in what situations this error occurs (or by that time you have a reproducible scenario:)).