Unwanted side effects in _formOptions

sliekens

Editor contains the following code:

// Start a full row edit, but don't display - we will be showing the field
this._edit( cell, editFields, 'inline' );
var namespace = this._formOptions( opts );

var ret = this._preopen( 'inline' );
if ( ! ret ) {
    return this;
}

I don't understand exactly what this._formOptions( opts ) does, but I think the call should happen after the preOpen event.

// Start a full row edit, but don't display - we will be showing the field
this._edit( cell, editFields, 'inline' );
var ret = this._preopen( 'inline' );
if ( ! ret ) {
    return this;
}
var namespace = this._formOptions( opts );

I forgot what issue this change fixed for me. All I remember is that _formOptions performs some side effect(s) that should not occur if _preopen returns false.

allan

The _formOptions method provides common handling for the various form types such as setting buttons, titles, etc. The fact that preOpen happens after it allows that event to override those options.

Allan

sliekens

That makes sense, but then why is the order of events different for bubble forms?

Inside Editor.prototype.bubble:

this._edit( cells, editFields, 'bubble' );

var ret = this._preopen( 'bubble' );
if ( ! ret ) {
    return this;
}

// Keep the bubble in position on resize
var namespace = this._formOptions( opts );

Inside Editor.prototype.inline:

// Start a full row edit, but don't display - we will be showing the field
this._edit( cell, editFields, 'inline' );
var namespace = this._formOptions( opts );

var ret = this._preopen( 'inline' );
if ( ! ret ) {
    return this;
}

I remember a little more about my problem:
The call to _formOptions attaches a handler for keydown events in the page. This conflicts with KeyTable (somehow), especially when the preOpen event is cancelled by user code.

I recall a similar issue in KeyTable that was fixed in the last release by adding a cancelOpen event. Maybe _formOptions should also listen for that event?

allan

That makes sense, but then why is the order of events different for bubble forms?

That's a bug - thanks for pointing it out. It should be consistent.

Yes, it sounds like preOpen being cancelled might be causing a problem in the bubble as well.

Allan

allan

Hi,

Quick update on this one. I've committed fixes for both issues and they will be in 1.6.3. Thanks again for letting me know about these!

Regards,
Allan