I don't understand why the Travis CI build failed, any ideas?

Travis says that your formatting is wrong:
FILE: ...home/travis/build/joomla/joomla-cms/plugins/editors/tinymce/tinymce.php

FOUND 2 ERROR(S) AFFECTING 2 LINE(S)

363 | ERROR | Expected "if (...)\n...{...}\n...else\n"; found

 |       | "if(...)\n...{...}\n...else "

371 | ERROR | Expected "if (...)\n...{...}\n...else\n"; found

 |       | "if(...)\n...{...}\n...else\n"

i.e. Instead of

You should have

(Note: Did not test patch itself)

Thanks! Fixed the whitespace issues.

This works, except that the test should be:

$editor = JEditor::getInstance('tinymce'); 
echo $editor->display('description', "", '250', '50', '60', '5', false);

$width comes before $height
See https://github.com/joomla/joomla-cms/blob/staging/libraries/cms/editor/editor.php#L289

I have tested this item ✅ successfully on 984c0c7

EDIT: breaks $html_height in params. Changed test to unsuccessful.

@Fedik @dgt41
Please test. I guess this can go in.

but the advanced params still not working

@AlexRed If those settings did not work before, I guess it is a separate issue. This fix is only for 3rd party components that programmatically display the editor.

Before your patch the "HTML Height" parameter work, after not work :(

I confirm @AlexRed findings and changed my test to negative.

The issue is that $height has always a value.
To test, just add a var_dump before checking height

		var_dump($height);
		die;
		if (isset($height))
		{
			$html_height = $height;
		}
		else
		{
			$html_height = $this->params->get('html_height', '550');
		}

$height will default to 500px

so, tho get this to work (for height only), we would have to use

                if ($height != 500)
		{
			$html_height = $height;
		}
		else
		{
			$html_height = $this->params->get('html_height', '550');
		}

isset is the wrong command and could not be used here. The code should be changed to:

if ($height)
{
	$html_height = $height;
}
else
{
	$html_height = $this->params->get('html_height', '550');
}

if ($width)
{
	$html_width  = $width;
}
else
{
	$html_width  = $this->params->get('html_width', '');
}

With this change, custom width (and height) if defined will be used instead of the width and height defined in plugins parameter

The easiest way to test is try to add a new article from backend of the site, try to edit this xml code https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_content/models/forms/article.xml#L27 , add width and height attributes (see https://docs.joomla.org/Editor_form_field_type for document about editor field type). For example, change it to;

<field name="articletext" type="editor"
			label="COM_CONTENT_FIELD_ARTICLETEXT_LABEL" description="COM_CONTENT_FIELD_ARTICLETEXT_DESC"
			filter="JComponentHelper::filterText" buttons="true"  height="300" width="300" />

1. Before the change, the custom with and height is not used. The width and height parameters defined in editor plugin is used

2. After changed, the custom height and width passed is used (note, size of editor changed)

3. Remove width, height attribute from XML code above, the editor again uses the size defined in the plugin parameter

Please note that in case the form has multiple editors, the width and height defined in the first displayed editors will be used. If you define difference size for different editor, it won't have any affect, the size defined in the first displayed editor always win.

@joomdonation
As I wrote above, we always have a default height of 500px, therefore it kills any height defined in the plugin params.

@infograf768 How did you test it? Please try to change the XML file of add article form as I suggested, you will see that it works

Do not change anything. Do not patch or if you patch, do the var_dump as I have shown above.

var_dump($height);
		die;

We do have a default 500px coming from I don't know where (maybe from tiny js files)

Ah, see your point. See https://github.com/joomla/joomla-cms/blob/staging/libraries/cms/form/field/editor.php#L204-L205

If we don't define width and height in the form field parameters, default values are passed there instead of using editor parameters. No sure what's the reasons

If we don't define width and height, default values are passed there instead of using editor parameters. No sure what's the reasons

Bingo. it was there

In this case, I think the best fix we could do is changing the code to:

if ($height && $height != '500px')
{
	$html_height = $height;
}
else
{
	$html_height = $this->params->get('html_height', '550');
}

if ($width && $width != '100%')
{
	$html_width  = $width;
}
else
{
	$html_width  = $this->params->get('html_width', '');
}

Not sure we need
if ($height && $height != '500px')

as $height always exists.

Also, no need for 500px. My test show that 500 is fine`

Not sure we need
if ($height && $height != '500px')

=> Yes, we need it to give more flexible to developer. For example, when you call

echo $editor->display('description', "", '', '', '60', '5', false);

The editor should be rendered using width and height from plugins parameter

Also, no need for 500px. My test show that 500 is fine`

When no height is being passed, $height value is set to 500px (it is a string). I guess your code works because php doing magic comparison. For example the code below display true when the two value actually not equals

var_dump(500 == '500px');

The editor should be rendered using width and height from plugins parameter

Are you sure it does not pick the default from

			$this->height      = $this->element['height'] ? (string) $this->element['height'] : '500';
			$this->width       = $this->element['width'] ? (string) $this->element['width'] : '100%';

It picks the default value if we are using editor form field type. It doesn't have default value when you call the $editor->display directly (in the code you gave for testing)

@DerinMavi Could you please look at my suggested changes and implement it so that we can re-test this PR?

I will try to fix it today.

@joomdonation : The problem with your suggestion is that if you set height: '500px' or width:'100%' in the article.xml , your settings will be ignored and they will be overridden by tinymce advanced settings parameters.

The root cause of the problem is that for tinymce there are 2 places where default width/height is provided. Once onDisplay method inside tinymce.php is called it is impossible to know whether the widht/height are default values or user set values.

For this reason, I removed the default values from setup() method in editor.php.

For all editors, default values are set inside onDisplay() methods.

Please @infograf768 and @AlexRed test the new fix.

@DerinMavi The reason I suggested it because I was worry that if we remove default values in setup() method (like how you did in recent commit), there might be backward compatible issue with third party editor plugins.

Right now, if no width and height provided in the XML form field, third party editor plugins might expect that default width and height provided by setup method and they might not deal with empty value (like you modified for the editor plugin shipped by Joomla). That's why I mentioned in my comment that best fix we could do

@joomdonation You are right, I checked couple of 3rd party editors and saw that they'd break if the defaults are not set. I finally settled with your solution. I changed != with !==(so that if someone passes 500 (integer) from the code it'd work as expected). Also changed the default value of 550px to 500px so that it is consistent with previous default value.

We had a good reason for if ($html_width == 750). Shall not we re-add it?

@Fedik @joomdonation So does it cause any problems if that check is not there? You found why it was added but that was before default settings were working. Now it should work as it supposed to be. If you leave the setting as it is(=750) during installation, that value will be used instead of default tinymce value.

Before this change, despise the set value of 750 it'd still use tinymce default setting, which is 550. That would also mean it would not work if the user really wanted to use width as 750 pixels.

TinyMCE sizing does not work with form fields either, and I don't mean the new ones..
name="brandText"
class=""
type="editor"
default=""
cols="2"
rows="3"
width="300px"
height="300px"
Will produce a fully screen layout

@N6REJ
width="300px"
height="300px"
Numeric! remove px and it should be ok
px is added later

Edit: Sorry, I now saw in the code that it adds px if numeric otherways leaves it as is.

I know the height works in a form as I use it.

I'm on programatically change the editor height, this is not fixed in Joomla 3.8.6 !
Now more then1 year for this ?
Come on guys, it's not a big deal to check for values and use right ones.

In /plugins/editors/tinymce/tinymce.php line 408

		if($height) $html_height = $height;
		else $html_height = $this->params->get('html_height', '550');

Set the height correctly.

Same for me testing instruction 1 and 2 works fine on my local machine, but the third instruction seems to fail. I found another layout issue please check the attached screenshot from the dropdown-button behaviour @icampus

I have tested this item ? unsuccessfully on d981c6b

I followed the instructions step by step but the patch didn't work well. Before i applied the patch i could change the editor size in the web config well but not in the xml file. After i applied the patch, i couldn't change anything, not in the web config neither nor in the xml File. So i thing the patch is not completely correct.

@icampus

Replaced by PR #26755