Dialog error when remove editor instance

[Garry Yao]

The following regression is seen after [5412]:

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.

[Alfonso Martínez de Lizarrondo]

Is it really expected for the people to destroy the editor while a dialog is open?

I can see that now there's an error when an instance is destroyed if no dialog was open, and can be fixed by a little patch, but all those changes to take care of the proposed situation seem a little overkill to me.

[Alfonso Martínez de Lizarrondo]

Simple patch

[Alfonso Martínez de Lizarrondo]

This patch fixes a regression that must be fixed ASAP, and then we can keep talking about the previous patch and if that's a situation that should be supported.

[Frederico Caldeira Knabben]

I'm ok with Alfonso's patch. It can be committed leaving this ticket opened. Please remove the review keyword after that.

[Frederico Caldeira Knabben]

Replying to alfonsoml:

Is it really expected for the people to destroy the editor while a dialog is open?

We can't say no for that, but it may happen.

I can see that now there's an error when an instance is destroyed if no dialog was open, and can be fixed by a little patch, but all those changes to take care of the proposed situation seem a little overkill to me.

Garry's proposal is not bad. It doesn't bring too many changes to the code, mainly changing the logic on the cover management, which is ok.

[Frederico Caldeira Knabben]

@Garry... I've reviewed your patch, and there is a problem with it, which is related to the z-index changes you've made.

If you open the table cell dialog, you can then open the color dialog for the cell background. Note that, after the patch, the second dialog cover is not there.

[Frederico Caldeira Knabben]

I've committed Alfonso's patch with [5448] because things are pretty broken without it.

[Garry Yao]

If you open the table cell dialog, you can then open the color dialog for the cell background. Note that, after the patch, the second dialog cover is not there.

Previously we made all opened dialogs model (by overlaying the latest opened upon the old one), while I'm proposing here to remove this feature, with the advantages of being able to edit all opened dialog simultaneously, does it make sense?

[Frederico Caldeira Knabben]

Replying to garry.yao:

If you open the table cell dialog, you can then open the color dialog for the cell background. Note that, after the patch, the second dialog cover is not there.

Previously we made all opened dialogs model (by overlaying the latest opened upon the old one), while I'm proposing here to remove this feature, with the advantages of being able to edit all opened dialog simultaneously, does it make sense?

That's not good, because it takes the use focus out of the "current task", causing confusion. Let's have the current behavior preserved.

[Garry Yao]

Fixed with [5476].