Ticket #5782 (closed Bug: fixed)

Opened 4 years ago

Last modified 4 years ago

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

Reported by: wwalc Owned by: garry.yao
Priority: Normal Milestone: CKEditor 3.3.1
Component: General Version: 3.3
Keywords: Confirmed Review+ Cc:

Description (last modified by wwalc) (diff)

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

FF_not_initialized_error.zip (25.0 KB) - added by wwalc 4 years ago.
5782.patch (1.5 KB) - added by garry.yao 4 years ago.
5782_2.patch (1.7 KB) - added by garry.yao 4 years ago.
5782_3.patch (2.1 KB) - added by garry.yao 4 years ago.

Change History

Changed 4 years ago by wwalc

comment:1 Changed 4 years ago by wwalc

  • Description modified (diff)

comment:2 Changed 4 years ago by wwalc

  • Description modified (diff)

Changed 4 years ago by garry.yao

comment:3 Changed 4 years ago by garry.yao

  • Keywords Review? added
  • Owner set to garry.yao
  • Status changed from new to assigned

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 4 years ago by alfonsoml

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

comment:5 Changed 4 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 4 years ago by fredck

  • Milestone changed from CKEditor 3.4 to CKEditor 3.3.1

comment:7 Changed 4 years ago by alfonsoml

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

comment:8 Changed 4 years ago by garry.yao

  • Keywords Review? added; Review- removed

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

comment:9 Changed 4 years ago by alfonsoml

  • Keywords Review+ added; Review? removed

comment:10 Changed 4 years ago by garry.yao

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

Fixed with [5566].

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