#9087 closed feature (fixed)
Dialog: Allow registering elements outside a dialog for use when a modal dialog is open
Reported by: | psyafter | Owned by: | psyafter |
---|---|---|---|
Priority: | minor | Milestone: | 1.10.2 |
Component: | ui.dialog | Version: | 1.10.0 |
Keywords: | Cc: | ||
Blocked by: | Blocking: |
Description (last modified by )
When using CKEditor in UI Dialog there is a problem with editor popups.
jsfiddle: http://jsfiddle.net/psyafter/dUThT/
1) click on button 2) in ckeditor double click on text "click me" 3) in editor dialog there is a problem edit textboxes
Solution
See comment #30 for an extension that can be loaded after jQuery UI to register CKEditor elements as valid targets.
Change History (39)
comment:1 Changed 10 years ago by
comment:2 follow-up: 3 Changed 10 years ago by
Owner: | set to psyafter |
---|---|
Status: | new → pending |
To be honest, this seems like a strange behavior to implement. Why are you putting a full editor inside a modal dialog? Do you have suggestions for how to sanely handle this?
comment:3 Changed 10 years ago by
Status: | pending → new |
---|
Replying to scott.gonzalez:
To be honest, this seems like a strange behavior to implement. Why are you putting a full editor inside a modal dialog? Do you have suggestions for how to sanely handle this?
CKEditor in ui dialog is because of requirements. In past versions of jquery-ui there was patch that worked about 2 years. link to patch: http://forum.jquery.com/topic/can-t-edit-fields-of-ckeditor-in-jquery-ui-modal-dialog#14737000001648387
But today, after jquery-ui-dialog (and overlay) has been redesigned - patch not work because of missing $.ui.dialog.overlay. I believe jquery-ui team can rewrite a patch (with existing ui core conventions)
comment:4 follow-up: 5 Changed 10 years ago by
Status: | new → pending |
---|
I believe jquery-ui team can rewrite a patch (with existing ui core conventions)
If I knew a solution, I wouldn't be asking you for one. We also wouldn't still be fighting z-index and stacking issues after several years. We have what we believe to be the best solution we've come up with so far, which is to append popups to the element they're coming from. Popping up any dialog from a modal dialog is bad UX, but anticipating the various ways that this is done from all other JS libraries isn't possible.
What if we allowed you to register a selector that would define elements which should override the modal behavior? Then you could manually register ".cke_dialog"
. It's hacky and I don't like it, but I think it would work. What do you think?
comment:5 Changed 10 years ago by
Status: | pending → new |
---|
Replying to scott.gonzalez:
What if we allowed you to register a selector that would define elements which should override the modal behavior? Then you could manually register
".cke_dialog"
. It's hacky and I don't like it, but I think it would work. What do you think?
1) for a start - which lib is responsiblee for preventing editing? jquery-ui or ckeditor? 2) your solution is something like "white list" where each registered element will not be parsed by jquery-ui-dialog functionality? (it will be nice solution)
comment:6 follow-up: 7 Changed 10 years ago by
which lib is responsiblee for preventing editing?
Clearly jQuery UI. You've asked a jQuery UI dialog to prevent editing of everything that is not inside the dialog. It's doing what you've asked.
your solution is something like "white list" where each registered element will not be parsed by jquery-ui-dialog functionality?
Correct.
comment:7 Changed 10 years ago by
Replying to scott.gonzalez: fine, thanks for responses. solution that you propose will be really useful.
comment:8 follow-up: 9 Changed 10 years ago by
In Google Chrome Version 24.0.1312.70 I was unable to reproduce your issue. I was able to edit the inputs in the link properties pop up just fine.
Were you experiencing this issue in a particular browser?
Ok just tried it in Firefox v 18.0.2 and I am able to reproduce the issue.
comment:9 Changed 10 years ago by
Replying to djQuery:
In Google Chrome Version 24.0.1312.70 I was unable to reproduce your issue. I was able to edit the inputs in the link properties pop up just fine.
In Google Chrome 24.0.1312.57m is reproduced too
comment:11 Changed 10 years ago by
I have the same bug. In which version will it be fixed? Updated to jquery ui 1.10.1, but the bug is still there.
comment:12 Changed 10 years ago by
Milestone: | none → 1.10.2 |
---|---|
Status: | new → open |
Summary: | ui dialog prevent editing inputs in CKEditor popups → Dialog: Allow registering elements outside a dialog for use when a modal dialog is open |
Type: | bug → feature |
I'm marking as valid, but we're not going to fix this directly in jQuery UI because we feel that this is bad UX. What we're willing to do is expose a method that can be overridden to allow an API like the one I suggested.
comment:13 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | open → closed |
Dialog: Extract check for which elements can gain focus into its own method for overriding. Fixes #9087 - Dialog: Allow registering elements outside a dialog for use when a modal dialog is open.
Changeset: 51eb28e76e372fe0af12724edff0b5780b5e5ed0
comment:14 follow-up: 15 Changed 10 years ago by
This change allows you to write an extension like so:
$.widget( "ui.dialog", $.ui.dialog, { _allowInteraction: function( event ) { return !!$( event.target ).closest( ".other-popups" ).length || this._super( event ); } });
comment:15 Changed 10 years ago by
Replying to scott.gonzalez:
This change allows you to write an extension like so:
very nice! when 1.10.2 will be released?
comment:16 Changed 10 years ago by
We don't set specific dates for releases, but we generally release about once per month.
comment:17 Changed 10 years ago by
I am still seeing the same bug with CKEditor and jqueryui version 1.10.2
comment:18 Changed 10 years ago by
@pmckensey The "bug" will continue to exist for all versions, unless you explicitly write an exception as mentioned above. See http://bugs.jqueryui.com/ticket/9087#comment:14
comment:19 Changed 10 years ago by
@scott.gonzalez, I haven't understood the 14th comment, need I do something? or the ckeditor team need to implement that inside ckeditor? please explain yourself a little more...
comment:20 Changed 10 years ago by
@altieres It's up to the CKEditor team whether they add code for this. The bottom line is that if you want to have something that isn't part of the DOM inside the dialog, you'll need to overwrite _allowInteraction()
to support whatever other elements you want. Take a look at the current implementation which hacks in support for datepicker to get an idea of how this works.
comment:23 Changed 9 years ago by
Whew! Took me forever to find that my problem could be fixed--thanks for posting this fix (in comment #14). But could someone please clarify how to use this with CKEditor or point to an example page? I am using CKEditor 4.2.1 and jQuery UI 1.10.3, and I have included a new js file after including jquery UI:
$.widget( "ui.dialog", $.ui.dialog, { _allowInteraction: function( event ) { return !!$( event.target ).closest( ".cke_dialog" ).length || this._super( event ); } });
This does not fix the issue for me, though. Even before adding this patch / fix, I could edit all non-text / non-textarea elements in my plugin dialog...
With this fix, I have also tried ".cke_dialog" and ".cke_dialog_ui_input_text", but neither works. The text is still non-editable, non-highlightable. Maybe I don't understand how this fix is supposed to work, so any clarification is appreciated. Thanks!
comment:24 Changed 9 years ago by
@faren451: You'll need to post on the forums or Stack Overflow for help with your specific case.
comment:25 Changed 9 years ago by
As an FYI this ticket is being referenced as the solution for everyone using CKEditor 4.x nested within a jQuery-UI modal. I literally have read through 50+ posts online that copy/pasted the result above and cite it as the solution to their problems.
Do not even bother copying and pasting the snippet above and expecting this to solve your problems - I tried - it doesn't and can't work. I investigated why, and I don't believe that the poster ever expected it to be cited as the solution to our problem, rather he likely expected the CKEditor people to provide some type of proper solution for the observed problem, which they have failed to provide as of today.
The hook above does provide hope, but it falls short of being a proper solution for a number of reasons:
- The base method likely provided functionality that you want
- The author of the proposed solution above is likely not familiar with CKEditor, and did not account for the nested iframe that CKEditor uses.
- Due to the implementation of CKEditor, clicks on the nested iframe in CKEditor will generate an event.target as a "document", instead of generating an event.target as an "element". A document is not an element, so the .closest() call will return zero elements. This means that all interaction with CKEditor will be explicitly blocked from that point on.
My solution is attached below, but I'm sure there is something wrong with it. If anyone has better ideas I'd love to hear them - I want to preserve as much of the native functionality as possible while allowing the nested CKEditor to work until I change over to the newly supported IPE functionality:
$.widget( "ui.dialog", $.ui.dialog, { _allowInteraction: function( event ) { if ( $( event.target ).closest(".ui-dialog").length ) { return true; } if (!(event.target instanceof HTMLElement)) { return true; } if ( $( event.target ).closest(".cke_dialog").length ) { return true; } // TODO: Remove hack when datepicker implements // the .ui-front logic (#8989) return !!$( event.target ).closest(".ui-datepicker").length; }, _moveToTop: function (event, silent) { if (!event || !this.options.modal) { this._super(event, silent); } } });
I realize this probably is not the best place for such a dialog, but the problem is being discussed on many different forums and there is no single place to note that the provided solution will not work out of the box for most implementations.
comment:26 Changed 9 years ago by
@kamelkev I think it's fine to have this conversation here. I'm even willing to update the body of the ticket to clearly show the best solution, once the community has agreed on a working solution.
A few suggestions:
Do not manually re-implement the built-in logic; you're killing the upgrade path if you do. Use this._super( event )
to determine if the built-in logic allows interaction. This would result in something like:
_allowInteraction: function( event ) { if ( this._super( event ) ) { return true; } // custom logic goes here }
Don't use instanceof
. While the reasons for avoiding instanceof
are generally tied to supporting cross-window usage, and you're explicitly trying to check for that case, it's not actually checking what you want to check. You could instead just check if event.target.ownerDocument
matches this.document[ 0 ]
to determine if the event originated in the same document.
Can you explain why you needed to proxy _moveToTop()
?
comment:27 Changed 9 years ago by
Hi Scott, thanks for commenting. I was afraid people had completely lost interest in this. Definitely looking for the "right" way to do it.
It was an accident to paste moveToTop here, but it is relevant to the conversation. Basically the moveToTop adjustment handles a previous issue the CKEditor/jQueryUI community had whereby some iframe populated drop downs would lose their contents the second time they were clicked. There are a bunch of solutions out there for handling the problem, I found this one to be the simplest. I have tested it on a bunch of browsers, but I do not have it in production yet.
Great suggestions, I reimplemented and started our team testing it on all common browsers. So far the following appears to meet all requirements:
$.widget( "ui.dialog", $.ui.dialog, { /*! jQuery UI - v1.10.2 - 2013-12-12 * http://bugs.jqueryui.com/ticket/9087#comment:27 - bugfix * allowInteraction fix to accommodate windowed editors */ _allowInteraction: function( event ) { if ( this._super( event ) ) { return true; } if ( event.target.ownerDocument != this.document[ 0 ] ) { return true; } if ( $( event.target ).closest( ".cke_dialog" ).length ) { return true; } }, /*! jQuery UI - v1.10.2 - 2013-10-28 * http://dev.ckeditor.com/ticket/10269 - bugfix * moveToTop fix to accommodate windowed editors */ _moveToTop: function ( event, silent ) { if ( !event || !this.options.modal ) { this._super( event, silent ); } } });
comment:28 Changed 9 years ago by
Great. Can you report back once you've had this in production for a while, or done rigorous in-house testing? Once you've confirmed this, I'll update the ticket description. Hopefully other users will test out your fix as well so they can report success/failure in their apps.
If you want to put together a reduced test case showing the iframe issue using just jQuery UI, we can get that portion of the fix into dialog as well. I just want to keep anything CKEditor-specific out of jQuery UI.
comment:29 Changed 9 years ago by
QA came back and found a problem with the functionality of CKEditor under IE8/9/10/11. Basically there are some drop downs within the primary toolbar that are misbehaving. Under Firefox the drop downs (which produce an iframe displaying the drop down content) were not displaying the second time, which is why I was applying the _moveToTop fix above, however there is apparently a similar problem under IE where the drop downs display properly, but immediately disappear after displaying.
I rolled back to 1.9.x and sure enough the problem doesn't exist there - so it's likely related to the same issues that were affecting other parts of the tool.
The good news is that this appears to be the last issue that needs resolution for me to attempt a production release. The bad news is that I have literally no idea how this display/hide event is occurring.
I'm a little surprised to be hearing about this now, as I can find no-one else complaining about this specific problem. Maybe nobody else got this far with getting CKEditor 4.x running within a 1.10.x jQuery UI modal, but I find that a little hard to believe.
From what I can tell the following series of events occurs:
- Click link activating the jQueryUI dialog
- Click link for drop down within jQueryUI dialog
- Upon clicking observe that the drop down did not appear, however appears the textarea is now selected
- Click link for drop down within jQueryUI dialog
- Upon clicking observe that the drop down now has appeared, however almost immediately it will disappear.
You can see from the above sequence that no event actually occurs which would cause a change in focus. It seems the drop down is disappearing after a timer goes off or something. I have no such timers in my code.
From here my next plan is to write up an isolated test on jsfiddle to attempt to get help finding a solution.
If anyone has ideas on what is causing this, would love to hear them. We are pretty close at having something viable.
comment:30 follow-up: 36 Changed 9 years ago by
Found a final workaround for the aforementioned issue. Anyone interested may visit the following fiddle with working CKEditor 4.3.1 within a jQueryUI 1.10.2 dialog. I had problems with the page scrolling all the way to the top with jQuery 1.10.3, issues that I do not plan to dig into right now.
Fiddle: http://jsfiddle.net/kamelkev/HU8Qt/3/
Code that we have tested in IE 8/9/10, and latest versions of autoupdated browsers (Firefox,Chrome,Safari):
$.widget( "ui.dialog", $.ui.dialog, { /*! jQuery UI - v1.10.2 - 2013-12-12 * http://bugs.jqueryui.com/ticket/9087#comment:27 - bugfix * http://bugs.jqueryui.com/ticket/4727#comment:23 - bugfix * allowInteraction fix to accommodate windowed editors */ _allowInteraction: function( event ) { if ( this._super( event ) ) { return true; } // address interaction issues with general iframes with the dialog if ( event.target.ownerDocument != this.document[ 0 ] ) { return true; } // address interaction issues with dialog window if ( $( event.target ).closest( ".cke_dialog" ).length ) { return true; } // address interaction issues with iframe based drop downs in IE if ( $( event.target ).closest( ".cke" ).length ) { return true; } }, /*! jQuery UI - v1.10.2 - 2013-10-28 * http://dev.ckeditor.com/ticket/10269 - bugfix * moveToTop fix to accommodate windowed editors */ _moveToTop: function ( event, silent ) { if ( !event || !this.options.modal ) { this._super( event, silent ); } } });
I am deploying this during the first week of January at which point I will report back again.
comment:31 Changed 9 years ago by
Ok I have had this in production for a few weeks now. We have fairly high volume where the above fix is deployed - everything seems working as expected.
comment:32 Changed 9 years ago by
Description: | modified (diff) |
---|
comment:33 Changed 9 years ago by
Thanks, I've updated the ticket body to guide any users who find this ticket directly to your patch.
comment:36 Changed 9 years ago by
Thanks a lot kamelkev for your input, this code snippet works like a charm. Hope this issue will get fixed in next jQueryUI release.
Do you have any idea on how to fix the page scrolling issue? When I open the editor in a dialog, it scrolls all the way up to the top of the page (jQuery 1.11.0, jQueryUI 1.10.4 and CKEditor 4.4.0).
(sorry to post here, couldn't find any open ticket related to this).
Thanks, M
Replying to kamelkev:
Found a final workaround for the aforementioned issue. Anyone interested may visit the following fiddle with working CKEditor 4.3.1 within a jQueryUI 1.10.2 dialog. I had problems with the page scrolling all the way to the top with jQuery 1.10.3, issues that I do not plan to dig into right now.
Fiddle: http://jsfiddle.net/kamelkev/HU8Qt/3/
Code that we have tested in IE 8/9/10, and latest versions of autoupdated browsers (Firefox,Chrome,Safari):
$.widget( "ui.dialog", $.ui.dialog, { /*! jQuery UI - v1.10.2 - 2013-12-12 * http://bugs.jqueryui.com/ticket/9087#comment:27 - bugfix * http://bugs.jqueryui.com/ticket/4727#comment:23 - bugfix * allowInteraction fix to accommodate windowed editors */ _allowInteraction: function( event ) { if ( this._super( event ) ) { return true; } // address interaction issues with general iframes with the dialog if ( event.target.ownerDocument != this.document[ 0 ] ) { return true; } // address interaction issues with dialog window if ( $( event.target ).closest( ".cke_dialog" ).length ) { return true; } // address interaction issues with iframe based drop downs in IE if ( $( event.target ).closest( ".cke" ).length ) { return true; } }, /*! jQuery UI - v1.10.2 - 2013-10-28 * http://dev.ckeditor.com/ticket/10269 - bugfix * moveToTop fix to accommodate windowed editors */ _moveToTop: function ( event, silent ) { if ( !event || !this.options.modal ) { this._super( event, silent ); } } });I am deploying this during the first week of January at which point I will report back again.
comment:37 Changed 8 years ago by
It seems that, since jQuery UI 1.11, the _moveToTop
override is no longer necessary (see #9166). Can anyone else confirm? If this is true, then the CKEditor workaround could be reduced to something like this:
$.widget( "ui.dialog", $.ui.dialog, { /** * jQuery UI v1.11+ fix to accommodate CKEditor (and other iframed content) inside a dialog * @see http://bugs.jqueryui.com/ticket/9087 * @see http://dev.ckeditor.com/ticket/10269 */ _allowInteraction: function( event ) { return this._super( event ) || // addresses general interaction issues with iframes inside a dialog event.target.ownerDocument !== this.document[ 0 ] || // addresses interaction issues with CKEditor's dialog windows and iframe-based dropdowns in IE !!$( event.target ).closest( ".cke_dialog, .cke_dialog_background_cover, .cke" ).length; } });
...I also wonder if the event.target.ownerDocument
check is necessary or redundant. Thoughts?
Tested with CKEditor 4.4.6 / jQuery UI 1.11.2 on IE9, Firefox 35, Chrome 40.
comment:38 Changed 8 years ago by
Hi,
I'm going to experiment and see if we can get 1.11.4 working. The last time I tried this I found that my fix was fundamentally flawed, but that it didn't work at all with 1.10.3 - hence why we only posted for 1.10.2
I'm really hoping the above comment is true, and I can just simplify the fix and move on.
comment:39 Changed 8 years ago by
Hi usmonster,
I agree with your assessment. The override for _moveToTop is no longer necessary. The initial fix from ticket http://bugs.jqueryui.com/ticket/10269 appears to be the only thing we need for this to work in jQuery UI 1.11.4. I noticed your _allowInteraction code is different, I cannot confirm if that works or not. The variant included below has been in production for me for 18 months, and I know for sure that works.
Fiddle: http://jsfiddle.net/kamelkev/80qex6v1/6/
Code that we have tested in IE 9/10/11, and latest versions of autoupdated browsers (Firefox,Chrome,Safari):
$.widget( "ui.dialog", $.ui.dialog, { /*! jQuery UI - v1.11.4 - 2015-06-05 * http://bugs.jqueryui.com/ticket/9087#comment:27 - bugfix * http://bugs.jqueryui.com/ticket/4727#comment:23 - bugfix * allowInteraction fix to accommodate windowed editors */ _allowInteraction: function( event ) { if ( this._super( event ) ) { return true; } // address interaction issues with general iframes with the dialog if ( event.target.ownerDocument != this.document[ 0 ] ) { return true; } // address interaction issues with dialog window if ( $( event.target ).closest( ".cke_dialog" ).length ) { return true; } // address interaction issues with iframe based drop downs in IE if ( $( event.target ).closest( ".cke" ).length ) { return true; } } });
past bugs reports that now any solution not work:
http://forum.jquery.com/topic/can-t-edit-fields-of-ckeditor-in-jquery-ui-modal-dialog http://bugs.jqueryui.com/ticket/4727