Opened 7 years ago

Closed 7 years ago

#5688 closed Bug (fixed)

UI dialog for the second instance fails

Reported by: Alfonso Martínez de Lizarrondo Owned by: Garry Yao
Priority: Normal Milestone: CKEditor 3.3
Component: UI : Dialogs Version: SVN (CKEditor) - OLD
Keywords: Confirmed Review+ Cc:

Description

Loading the ui color samples, the dialog that it's launched in second place to change the ui color lacks the color picker.

Attachments (2)

5688.patch (6.4 KB) - added by Alfonso Martínez de Lizarrondo 7 years ago.
Proposed patch
5688_2.patch (35.8 KB) - added by Garry Yao 7 years ago.

Download all attachments as: .zip

Change History (12)

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

Regression of [5412]

Changed 7 years ago by Alfonso Martínez de Lizarrondo

Attachment: 5688.patch added

Proposed patch

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

Keywords: Review? added
Milestone: CKEditor 3.4
Owner: set to Alfonso Martínez de Lizarrondo
Status: newassigned

At least we must fix it for 3.4, although I think that it's safe (and better) to fix it in 3.3

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

Keywords: Review? removed
Milestone: CKEditor 3.4CKEditor 3.3

This is worse than the initial description: any 'html' element in a dialog will be reused in the second instance if they are managed with the html id and getElementById.

Example: launch the image dialog in one instance, now open it again in the second instance and the lock and reset size buttons don't work.

We must look at every "type : 'html'," definition in the dialogs.

Changed 7 years ago by Garry Yao

Attachment: 5688_2.patch added

comment:4 Changed 7 years ago by Garry Yao

Keywords: Confirmed Review? added

As pointed by Alfonso, we should eliminate all DUP ids inside dialogs, I've grabbed them with this tool scripts:

void($('[id]').each(function(){
  var ids = $('[id='+this.id+']');
  if(ids.length>1 && ids[0]==this)
    console.log( this.id );
}));

comment:5 Changed 7 years ago by Garry Yao

The chunks of changes in 'image' plugin were actually moving function scope instead of actually change anything.

comment:6 Changed 7 years ago by Garry Yao

Owner: changed from Alfonso Martínez de Lizarrondo to Garry Yao
Status: assignednew

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

Keywords: Review- added; Review? removed

Can you fix at the same time the little typo in "smiley_emtions_label"?

And the changes in the flash dialog could be just to remove those ids (without adding new ones as they aren't used) and put the class to the second div. In fact, I'm not sure that the first div is used at all, but that's another bug.

'<div style="display:none"><div class="loading">&nbsp;</div></div>' +
'<div class="FlashPreviewBox"></div></div>';

In the image dialog, the preview doesn't work in the second instance

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

Keywords: Review+ added; Review- removed

Sorry, I got confused and didn't test the correct files.

comment:9 Changed 7 years ago by Garry Yao

Status: newassigned

Fixed with [5504].

comment:10 Changed 7 years ago by Garry Yao

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