Opera: using enter key in dialogs triggers javascript error

[Wiktor Walc]

When pressing enter in the dialog, an error occurs

Steps to reproduce (1)

• select some text
• open link dialog
• type some url and press enter (without moving with a tab to the "ok" button)
• result:

  Event thread: change
  Error:
  name: TypeError
  message: Statement on line 2255: Cannot convert undefined or null to Object
  stacktrace:   Line 2255 of linked script http://127.0.0.1/ckeditor%203.0.x/_source/plugins/dialog/plugin.js
      			return this.getInputElement().getValue();
    Line 72 of linked script http://127.0.0.1/ckeditor%203.0.x/_source/plugins/dialogui/plugin.js
      function(){ this.fire( 'change', { value : this.getValue() } ); }
    ...  Line 150 of linked script http://127.0.0.1/ckeditor%203.0.x/_source/core/event.js
      						listenerFunction.call( scopeObj, ev );
    ...  Line 245 of linked script http://127.0.0.1/ckeditor%203.0.x/_source/core/event.js
      								var retData = listeners[i].call( this, editor, data, stopEvent, cancelEvent );
    Line 48 of linked script http://127.0.0.1/ckeditor%203.0.x/_source/core/dom/domobject.js
      				domObject.fire( eventName, new CKEDITOR.dom.event( domEvent ) );
    ... 
  

[Garry Yao]

Could you explain what's happening here?

[Minh Nguyen]

In this bug, Opera try to access url field when popup dialog has hidden.

[Garry Yao]

We've seen similar bugs before, those are #3794, #4101, etc...
Minh's patch definitely works to eliminate this bug, while I'm smelling something wrong from the dialog hiding logic, [ ​http://dev.fckeditor.net/browser/CKEditor/trunk/_source/plugins/dialog/plugin.js#L807

where] the element will be detached from DOM tree, this's weird actually, never seen [any ​http://www.extjs.com/deploy/dev/examples/window/hello.html] ​other ​dialog implementation doing that, simply toggle the dialog element gain performance benefit as well.

[Garry Yao]

We need hear some voices here.

[Alfonso Martínez de Lizarrondo]

If you test with the ajax sample you'll see that now every dialog that has been launched will remain hidden in the page body even after the editor is destroyed.

I (mistakenly) thought that the current behavior was to hide the dialogs instead of destroying them, it just makes sense to avoid creating the dom again. I don't know why it's done this way.

[Frederico Caldeira Knabben]

Replying to garry.yao:

We need hear some voices here.

I'm ok to have the dialogs hidden only. I think the original intention was to keep the DOM clean, but we should not have such a drastic impact with it.

Alfonso's notes must be considered though.

[Garry Yao]

Fixed with [5412].

[Garry Yao]

The following regression is seen after the fix:

Reproducing Procedures

1. Load the 'Ajax' sample page and click 'Create Editor' button;
2. Open any of the the dialog;
3. Execute the following lines of JavaScript in global scope:

   removeEditor();
   

4. Click once again 'Create Editor' and open the same dialog;
   • Actual Result: Dialog opened with no viewport cover.

[Garry Yao]

The patch providing the ability for caching dialog covers.

[Frederico Caldeira Knabben]

Please DO NOT reuse tickets for regressions, opening new tickets for them, possibly pointing to the changesets that introduced them.

[Garry Yao]

#5618 is opened for it.

[Alfonso Martínez de Lizarrondo]

This has caused #5688