Opened 10 years ago
Last modified 8 years ago
#13612 review Bug
[mathjax] long formula causes dialog window to go out of the viewport
Reported by: | Wiktor Walc | Owned by: | kkrzton |
---|---|---|---|
Priority: | Nice to have (we want to work on it) | Milestone: | |
Component: | UI : Dialogs | Version: | 4.3 |
Keywords: | Cc: |
Description (last modified by )
Found on Chrome.
- Make sure the browser width is like 1000px or so.
- Enter the following formula in the mathjax dialog:
x = {-b \pm \sqrt{b^2-4ac} \over 2a}{-b \pm \sqrt{b^2-4ac} \over 2a}{-b \pm \sqrt{b^2-4ac} \over 2a}{-b \pm \sqrt{b^2-4ac} \over 2a}{-b \pm \sqrt{b^2-4ac} \over 2a}{-b \pm \sqrt{b^2-4ac} \over 2a}{-b \pm \sqrt{b^2-4ac} \over 2a}{-b \pm \sqrt{b^2-4ac} \over 2a}{-b \pm \sqrt{b^2-4ac} \over 2a}{-b \pm \sqrt{b^2-4ac} \over 2a}{-b \pm \sqrt{b^2-4ac} \over 2a}{-b \pm \sqrt{b^2-4ac} \over 2a}{-b \pm \sqrt{b^2-4ac} \over 2a}{-b \pm \sqrt{b^2-4ac} \over 2a}{-b \pm \sqrt{b^2-4ac} \over 2a}
- See that the Ok/Cancel buttons are unavailable.
Attachments (1)
Change History (21)
Changed 10 years ago by
Attachment: | Screen Shot 2015-07-31 at 13.15.53.png added |
---|
comment:1 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:2 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:3 Changed 10 years ago by
Status: | new → confirmed |
---|
comment:4 Changed 9 years ago by
Milestone: | → CKEditor 4.5.7 |
---|
comment:5 Changed 9 years ago by
Owner: | set to kkrzton |
---|---|
Status: | confirmed → assigned |
comment:6 Changed 9 years ago by
Status: | assigned → review |
---|
comment:8 Changed 9 years ago by
Status: | review → review_failed |
---|
- It's a dialog only issue, so it's a dialog responisbility to provide a fix/proper logic for that.
Proposed fix is within the general use function, which is unnecessary. It could be simply added to the dialog's proper field onLoad, like keyup listener here.
BUT here another fix would be applicable... :) we should not only listen for keyboard events, but any change to the texarea, e.g. paste event (using mouse) so
keyup
is not necessarily the best event here.
Please fill a new issue for MathJax dialog not updating on mouse paste and reference this ticket. Then go back to current ticket and the change existing event listener - once it gets R+ it will fixed related ticket as well.
- Could I also ask you for a minor refactorization here? It would be sweet to extract
!( CKEDITOR.env.ie && CKEDITOR.env.version == 8 )
to variable likeisSupportedBrowser
, it'd improve readability.
comment:9 Changed 9 years ago by
Status: | review_failed → assigned |
---|
comment:10 Changed 9 years ago by
Ad. 2
I agree it is a dialog only issue but listening on input is not enough. There is a case when preview can change without any prior events trigger. It happens when already inserted formula is double clicked in editor. The dialog is opened and textarea updates with focused equation. No input events are trigerred. So relayouting should also happen when values are changed programmatically.
The second thing is that we assume this is a dialog only issue and dialog is responsible for its size. The size is really controlled by content of the dialog, in this case mathjax iframe. It indirectly sets dialog width by setting preview iframe size.
So to be consistent we should assume one:
- content can control dialog size (without any further steps - it works now this way)
- content can control dialog size but must inform dialog about changes in its size (so e.g. the dialog can relayout)
- content can ask dialog to resize (but is's up to dialog to decide if it can be resized)
- dialog watches for changes in content and manages its size
My favorite is 2 because it is flexible and partially same as current behavior but gives a dialog additional possibilities to react to changes (and here i assume mathjax content can inform dialog after resizing).
It looks somehow complicated as for such simple issue but it's worth considering how it should work.
comment:11 Changed 9 years ago by
The case with editing formula mentioned above could be also solved with simple layout() call after updating dialog content. It is little less consistent but much simpler so I'll probably go in this direction:)
comment:12 Changed 9 years ago by
So I found the exact case which bothered me and why relayouting dialog after mathajx update event makes sense (also previously written test case fails because of that).
For the solution proposed in comment:8 the case looks like this:
Normally mathjax rendering is synchronous so calling dialog.layout() in input listener after setting new value (which also means rerendering) works and executes in proper order: rerender -> relayout. The problem is when mathjax dialog is opened for the first time. Mathjax lib/iframe is loaded in asynchronous manner. In this case when the above long formula is pasted and mathjax iframe is not already loaded, layouting happens before rendering preview (preview waits for mathjax to complete loading).
This case can be observed easily (with proposed fixes ofc) using throttling option in Chrome Devtools (network tab). For my localhost test it occurs up to DSL(2Mb/s 5ms RTT). To reproduce:
- Open mathjax dialog
- As soon as it opens paste long formula
It is really the edge case. If we would like to solve this I will go with comment:10 as a solution if not comment:8 solution is sufficient.
comment:13 Changed 9 years ago by
Status: | assigned → review |
---|
comment:14 Changed 9 years ago by
Milestone: | CKEditor 4.5.7 → CKEditor 4.5.8 |
---|
comment:15 Changed 9 years ago by
Milestone: | CKEditor 4.5.8 → CKEditor 4.5.9 |
---|
comment:16 Changed 9 years ago by
Milestone: | CKEditor 4.5.9 → CKEditor 4.5.10 |
---|
comment:17 Changed 9 years ago by
Milestone: | CKEditor 4.5.10 → CKEditor 4.5.11 |
---|
Moving tickets to the next milestone.
comment:18 Changed 9 years ago by
Milestone: | CKEditor 4.5.11 → CKEditor 4.6.1 |
---|
comment:19 Changed 8 years ago by
Milestone: | CKEditor 4.6.1 → CKEditor 4.6.2 |
---|
Moving to 4.6.2 minor release, as 4.6.1 is mostly about polishing 4.6.0.
comment:20 Changed 8 years ago by
Milestone: | CKEditor 4.6.2 |
---|---|
Priority: | Normal → Nice to have (we want to work on it) |
Changes in t/13612
This fix adds relayouting of the dialog whenever mathajx preview is rendered. It means dialog always will be centered unless user moved it manually. When dialog was moved manually, its' coordinates are not overwritten so that the user does not lose controll over positioning the dialog.