Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mentions panel is positioned in relation main window only and not inner scrollable element #3848

Open
jswiderski opened this issue Feb 10, 2020 · 0 comments
Labels
plugin:mentions The plugin which probably causes the issue. status:confirmed An issue confirmed by the development team. support:2 An issue reported by a commercially licensed client. type:bug A bug.

Comments

@jswiderski
Copy link
Contributor

Type of report

Bug

Provide detailed reproduction steps (if any)

  1. Create CKEditor inside scrollable container (div) and put those into scrollable page (e.g. body height 10000px)
  2. Open mentions panel and try to scroll inner element as well as the main page

Expected result

Mentions panel is glued to editor in both cases.

Actual result

Mentions panel is only glued to editor when main window is scrolled.

issue3

Other details

  • Browser: Any
  • OS: Any
  • CKEditor version: 4.10+
  • Installed CKEditor plugins: mentions
@jswiderski jswiderski added type:bug A bug. plugin:mentions The plugin which probably causes the issue. support:1 An issue reported by a commercially licensed client. labels Feb 10, 2020
@jacekbogdanski jacekbogdanski added the status:confirmed An issue confirmed by the development team. label Feb 11, 2020
@lslowikowska lslowikowska added support:2 An issue reported by a commercially licensed client. and removed support:1 An issue reported by a commercially licensed client. labels Feb 18, 2020
bblessmann pushed a commit to kivitendo/kivitendo-erp that referenced this issue Feb 16, 2022
Der Bug ist bekannt bei ckeditor: ckeditor/ckeditor4#3848

Was da passiert ist, dass das Plugin floatingspace, was benutzt wird um
die toolbar zu implementieren, den scroll handler nur auf document.body
macht, nicht aber auf scrollbare fivs, wie bei uns verwendet werden.

Der Fix im Quellcode vom ckeditor ist dann effektiv dieser diff hier:

diff --git a/plugins/floatingspace/plugin.js b/plugins/floatingspace/plugin.js
index 8abbcd61d..65f16025b 100644
--- a/plugins/floatingspace/plugin.js
+++ b/plugins/floatingspace/plugin.js
@ -26,6 +37,7 @@

        function attach( editor ) {
                var config = editor.config,
+               parent = parentScroll(editor.element),

                        // Get the HTML for the predefined spaces.
                        topHtml = editor.fire( 'uiSpace', { space: 'top', html: '' } ).html,
@@ -312,6 +324,7 @@
                                editor.on( 'change', changeBuffer.input );
                                win.on( 'scroll', uiBuffer.input );
                                win.on( 'resize', uiBuffer.input );
+                               parent.forEach(function(element) {
+                                 element.on( 'scroll', uiBuffer.input );
+                               });
                        } );

                        editor.on( 'blur', function() {
@@ -319,11 +332,13 @@
                                editor.removeListener( 'change', changeBuffer.input );
                                win.removeListener( 'scroll', uiBuffer.input );
                                win.removeListener( 'resize', uiBuffer.input );
+                               parent.forEach(function(element) {
+                                 element.removeListener( 'scroll', uiBuffer.input );
+                               });
                        } );

                        editor.on( 'destroy', function() {
                                win.removeListener( 'scroll', uiBuffer.input );
                                win.removeListener( 'resize', uiBuffer.input );
+                               parent.forEach(function(element) {
+                                 element.removeListener( 'scroll', uiBuffer.input );
+                               });
                                floatSpace.clearCustomData();
                                floatSpace.remove();
                        } );

Das sucht sich beim attachen das Ziel Element, sucht sich das/die
parent-Elemente die scrollbar sind, und registrtiert den scroll handler
auf diesen Elementen.

Leider gibt es keine Standardfunktion parentScroll() in javascript. Es
gibt ein Plugin gleichen Namens für jQuery, die Funktion da liefert aber
nur den nächst-höheren div und ausserdem ist jQuery innerhalb von
CKEditor nicht verfügbar. Die hier implementierte Funktion ist identisch
mit dieser hier, die angelehnt ist an das jQuery verhalten, aber pures
Javascript benutzt:

 function parentScroll(e) {
   var position = e.$.getAttribute("position"),
       excludeStaticParent = position === "absolute";
   return e.getParents().filter( function(parent) {
     var style = window.getComputedStyle(parent.$);
     if ( excludeStaticParent && style.position === "static" )
       return false;
     return (/(auto|scroll)/).test( style['overflow'] + style["overflow-y"] + style["overflow-x"] );
   });
 };

Im Gegensatz zum jQuery Plugin macht diese Variante keine Prüfung darauf
ob der Invokant selber "fixed" position hat, und liefert nicht nur
das erste Element sondern alle Treffer. Das Ergebnis ist ein normales
Javascript Array.

Der Code hier ist minified und direkt in unsere Variante injiziert, muss
also beim Upgrade reproduziert werden.

Was hier nicht gefixt ist: Das Repositioning Verhalten von floatingspace
orientiert sich weiter an Body, und nicht (wie eigentlich korrekt) am
umliegenden scrolling div.

(cherry picked from commit b64d03b5506441aa25433c0bb63753f5a126a675)
(cherry-pick von odyn)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin:mentions The plugin which probably causes the issue. status:confirmed An issue confirmed by the development team. support:2 An issue reported by a commercially licensed client. type:bug A bug.
Projects
None yet
Development

No branches or pull requests

3 participants