Skip to content

[6.1] Fix none/codemirror editor width and height#47305

Open
RickR2H wants to merge 7 commits intojoomla:6.1-devfrom
RickR2H:fix-none-editor
Open

[6.1] Fix none/codemirror editor width and height#47305
RickR2H wants to merge 7 commits intojoomla:6.1-devfrom
RickR2H:fix-none-editor

Conversation

@RickR2H
Copy link
Member

@RickR2H RickR2H commented Mar 5, 2026

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

  1. Go to System → Mail Templates. In the Options (top right of the screen), set Mail Format to Both.
  2. 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.
  3. 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"
/>
  1. 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.
  2. Check if the editor is respecting the TinyMCE plugin settings in the frontend of the site.

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

@brianteeman
Copy link
Contributor

brianteeman commented Mar 5, 2026

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

@RickR2H
Copy link
Member Author

RickR2H commented Mar 5, 2026

@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.

@brianteeman
Copy link
Contributor

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.

@Fedik Fedik linked an issue Mar 6, 2026 that may be closed by this pull request
@brianteeman
Copy link
Contributor

thank you @HLeithner

@brianteeman
Copy link
Contributor

FYI the same problem can be seen when editing content in the front end

image

@RickR2H
Copy link
Member Author

RickR2H commented Mar 7, 2026

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...

@brianteeman
Copy link
Contributor

brianteeman commented Mar 7, 2026

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

@Fedik
Copy link
Member

Fedik commented Mar 7, 2026

Any other suggestions?

I think it is good as it is, as in the PR.

@brianteeman
Copy link
Contributor

@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.

@Fedik
Copy link
Member

Fedik commented Mar 7, 2026

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?

@brianteeman
Copy link
Contributor

brianteeman commented Mar 7, 2026

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.

$theme = 'silver';

// Reset the old default 750px width of the editor to 100%
if (!$width || $width === '750px' || $width === '750') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should become obsolete if PR #47326 gets accepted.

@richard67 richard67 changed the title Fix none/codemirror editor width and height [6.1] Fix none/codemirror editor width and height Mar 7, 2026
@brianteeman
Copy link
Contributor

brianteeman commented Mar 7, 2026

Dont really want to create a new issue but this is confusing or another error? I can create a new issue if you prefer

image

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

@Fedik
Copy link
Member

Fedik commented Mar 7, 2026

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[6.1] Mail Templates Regression

6 participants