[#47305] - [6.1] Fix none/codemirror editor width and height
- Pending
- Medium
- Build: 6.1-dev
- # 47305
- Diff
- RickR2H:fix-none-editor
User tests: Successful: Unsuccessful:
Pull Request resolves #47292 .
This fixes a width and height bug introduced in PR #46438 when there is no editor loaded or editor is set to CodeMirror .
Summary of Changes
PR #46438 fixed the width and height parameters for TinyMCE. The issue now is that when no editor (editor set to 'none') or CodeMirror is used, the editor collapses because no default width or height is provided.
This occurs, for example, when editing email templates.
Testing Instructions
Testing instructions are in the original PR #46438.
Additional Testing Instructions
- Go to System → Mail Templates. In the Options (top right of the screen), set Mail Format to Both.
- Open a mail template and check whether the first editor field (where no editor is set) is full width and has a reasonable working height. Also check whether the TinyMCE editor below respects the width and height settings configured in the TinyMCE plugin.
- In addition to modifying the modules/mod_custom/mod_custom.xml file, add the code below to the XML after the background image field to have all editor options set in the XML file.
<field
name="editor1"
type="editor"
label="Editor set to CodeMirror"
height="600"
width="600"
editor="codemirror"
/>
<field
name="editor2"
type="editor"
label="Editor set to None"
height="300"
width="800"
editor="none"
/>
<field
name="editor3"
type="editor"
label="Editor set to TinyMCE"
height="900"
width="800"
editor="tinymce"
/>
<field
name="editor4"
type="editor"
label="Use default editor"
/>
- Switch the default editor in the configuration to TinyMCE, CodeMirror or None, and check if all editor fields are working accordingly in all places in the site.
- Check if the editor is respecting the TinyMCE plugin settings in the frontend of the site.
NOTE:
When upgrading from a previous installation, the editor width and height may still be set to the old default of 750px, which can cause the editor to appear too small. With PR #47326, the default editor dimensions are updated to width 100% and height 550px to ensure the editor displays at the correct size after the upgrade.
When testing this PR make sure to set the width to 100% and the height to 550px in the TinyMCE plugin.
Actual result BEFORE applying this Pull Request
The 'None' editor and CodeMirror editor is collapsed.
Expected result AFTER applying this Pull Request
The 'None' editor and CodeMirror editor is full width and has a usable working height.
Link to documentations
Please select:
-
Documentation link for guide.joomla.org:
-
No documentation changes for guide.joomla.org needed
-
Pull Request link for manual.joomla.org:
-
No documentation changes for manual.joomla.org needed
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Libraries |
@brianteeman Simply saying that something is wrong is not very constructive.
Maybe a solution would be to check in the individual plugins for the width and height values, as @Fedik suggested. If they are empty us a fallback for the width an height. Could that be a possible solution?
Otherwise I could ask the maintainers to revert my PR. In that case we would be left with an editor that, in my opinion, is not high enough to be practical, and with width and height plugin settings that effectively do nothing. That would also mean users are essentially forced to rely on an external editor plugin by default.
| Labels |
Added:
PR-6.1-dev
|
||
I am not saying that there is not an issue that needs to be addressed. I am saying that the changes that were made introduced new issues and blaming the user (as I was blamed) for not knowing about an undocumented change that they needed to make is not a suitable solution nor is it a solution to hard code band aid fixes as in this pr.
| Category | Libraries | ⇒ | Libraries Front End Plugins |
| Category | Libraries Front End Plugins | ⇒ | Front End Plugins |
HLeithner
-
FYI the same problem can be seen when editing content in the front end
Maybe we should remove the width parameter in total and show the editor always 100%. That has been the case in the past. And then just keep the height.
Other option is to remove the current parameters and introduce two new ones.
Any other suggestions?
As this is now a blocker we need a solution quick...
revert the pr that broke everything and then spend the time to write a complete solution that works correctly. The behaviour this was "fixing" is not new so its not desperate to fix that. Better to get it correct now instead of constantly adding band aid fixes on top
Any other suggestions?
I think it is good as it is, as in the PR.
@Fedik sad that you think so - at first you denied there was even a problem and it was my faulty for not knowing I should change an undocumented setting to make my site work. Thankfully Harald understood and set this as a release blocker. now you want to stick a band aid on it to fix the problem even though you are not fixing the problem you are just pretending that a setting in the plugin should be ignored if its set to 750 but accepted if its set to 749 or 751.
at first you denied there was even a problem and it was my faulty for not knowing I should change an undocumented setting to make my site work
Where I said it is your problem and not a bug?
now you want to stick a band aid on it to fix the problem even though you are not fixing the problem
It is a proper fix, with fallback to default values. The TinyMCE a bit tricky, but I am fine with that.
If you know how to update plugin parameters during upgrade and want to help then you are free to contribute to this PR. Until then the fix is good.
if its set to 750 but accepted if its set to 749 or 751.
You want to fight for 1px, seriously?
You completely miss the point but that doesnt suprise me at all.
Plugin paramaters can be updated in a PR - we've done it before just look and you will see, I woujld expect a maintainer to know this
Again you miss the point - this PR replaces 750 woith 100% but if its 749 or 751 then it is left as it is.
richard67
- | Title |
|
||||||
Dont really want to create a new issue but this is confusing or another error? I can create a new issue if you prefer
PLG_TINY_FIELD_HTMLHEIGHT_DESC="Set the height of the editor. Leave empty for automatic height. A numeric value can be used or a value with valid CSS units (for example: 50vh or 50rem)."
PLG_TINY_FIELD_HTMLWIDTH_DESC="Set the width of the editor. Leave empty for 100% width. A numeric value can be used or a value with valid CSS units (for example: 50vw or 50em)."
You cannot leave it as empty. If you delete the contents of the field then on save the values of 550px and 100% height are inserted
chrome_bvOMUWs7DH.mp4
Yes, the description is misleading, and never worked like that because the form has a default values.
Please open a new issue or PR for the description.
We can make work like that, but then we should remove default values from the from.
In any case that will be a different issue/fix.
| Labels |
Added:
Release Blocker
bug
|
||
46438 silently changed the width of tinymce on NEW installs only with no documentation telling existing users why their editor width is now wrong. Mail templates is just an easy visual indicator of this break. this pr just ignores that. the original pr was simply wrong and all you are doing now is trying to put a band aid on the error. and tomorrow you will need to put a band aid on another area that was broken by the fauly approach in 46438