Opened 7 years ago

Closed 7 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 7 years ago.
5782.patch (1.5 KB) - added by Garry Yao 7 years ago.
5782_2.patch (1.7 KB) - added by Garry Yao 7 years ago.
5782_3.patch (2.1 KB) - added by Garry Yao 7 years ago.

Download all attachments as: .zip

Change History (14)

Changed 7 years ago by Wiktor Walc

comment:1 Changed 7 years ago by Wiktor Walc

Description: modified (diff)

comment:2 Changed 7 years ago by Wiktor Walc

Description: modified (diff)

Changed 7 years ago by Garry Yao

Attachment: 5782.patch added

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

Attachment: 5782_2.patch added

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

Milestone: CKEditor 3.4CKEditor 3.3.1

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

Attachment: 5782_3.patch added

comment:8 Changed 7 years ago by Garry Yao

Keywords: Review? added; Review- removed

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

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

Keywords: Review+ added; Review? removed

comment:10 Changed 7 years ago by Garry Yao

Resolution: fixed
Status: assignedclosed

Fixed with [5566].

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