Opened 10 years ago

Closed 10 years ago

#5782 closed Bug (fixed)

FF: 0xc1f30001 (NS_ERROR_NOT_INITIALIZED) [nsIDOMNSHTMLDocument.designMode]

Reported by: Wiktor Walc Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.3.1
Component: General Version: 3.3
Keywords: Confirmed Review+ Cc:

Description (last modified by Wiktor Walc)

This is a regression bug introduced in CKEditor 3.3. It occurs when doing some specific DOM operations on the node that contains the editor. We had a similar problem in #5660.

Please check the attached files (unpack them in the _samples directory) to reproduce this issue. Works fine in CKEditor 3.2.1, 3.0 and all previous releases.

Attachments (4)

FF_not_initialized_error.zip (25.0 KB) - added by Wiktor Walc 10 years ago.
5782.patch (1.5 KB) - added by Garry Yao 10 years ago.
5782_2.patch (1.7 KB) - added by Garry Yao 10 years ago.
5782_3.patch (2.1 KB) - added by Garry Yao 10 years ago.

Download all attachments as: .zip

Change History (14)

Changed 10 years ago by Wiktor Walc

comment:1 Changed 10 years ago by Wiktor Walc

Description: modified (diff)

comment:2 Changed 10 years ago by Wiktor Walc

Description: modified (diff)

Changed 10 years ago by Garry Yao

Attachment: 5782.patch added

comment:3 Changed 10 years ago by Garry Yao

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

The DOM operations could be reduced to only CSS 'display' change, we need to understand that there's no way for us to know such change is happened in the parent tree, so programming in a defensive style helps.

comment:4 Changed 10 years ago by Alfonso Martínez de Lizarrondo

Keywords: Review- added; Review? removed

I have two questions/doubts with the patch:

The first function adds a

editor.document.getBody().focus(); 

at the end. Is it really needed? Other tickets have tried to avoid changing the focus as long as it's possible, so in order to have those issues back it would be better to know if it's needed and the drawbacks.

The second function finish with a call to blinkCursor() again. I think that the idea is that: function 1 fails, then function 2 is executed and then calling function 1 will work. Do we know that it's true and it won't start an endless loop of function 1 failing and executing function 2 over and over again?

Changed 10 years ago by Garry Yao

Attachment: 5782_2.patch added

comment:5 Changed 10 years ago by Garry Yao

Keywords: Review? added; Review- removed

Review of quality, the focus body is necessary to avoid having a dumb cursor when clicking on non-body area, for safety, I've included a anti dead-loop check.

comment:6 Changed 10 years ago by Frederico Caldeira Knabben

Milestone: CKEditor 3.4CKEditor 3.3.1

comment:7 Changed 10 years ago by Alfonso Martínez de Lizarrondo

Keywords: Review- added; Review? removed

I didn't realize that it's replacing a call to doc.getBody().focus(), and the other place where it's being called seems to work correctly.

Anyway, the patch creates a new problem: load the test case or just replacebyclass, and without clicking or focusing the content, just press the Source button. It generates this error:

editor.document is null
http://localhost/ckeditor/_source/plugins/wysiwygarea/plugin.js
Line 832

Changed 10 years ago by Garry Yao

Attachment: 5782_3.patch added

comment:8 Changed 10 years ago by Garry Yao

Keywords: Review? added; Review- removed

That's a careful catch while pretty an edge case though.

comment:9 Changed 10 years ago by Alfonso Martínez de Lizarrondo

Keywords: Review+ added; Review? removed

comment:10 Changed 10 years ago by Garry Yao

Resolution: fixed
Status: assignedclosed

Fixed with [5566].

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