?
avatar PhilETaylor
PhilETaylor
27 Mar 2017

Steps to reproduce the issue

Install from joomla-cms/staging
Due to bugs in the HEAD Im using commit 1f89fb0 for this test

Add an editor custom field in com_fields

Edit a content item, click fields tab, see the new editor, then attempt to use an editor plugin from the bottom row

screen shot 2017-03-27 at 16 24 40

Expected result

The features work as expected, and insert their results into the Editor Custom Field area

Actual result

The features popup into a modal, and on selecting their item, the modal stays shown and the results are inserted into the editor on the Content Tab and NOT into the new editor on the field tab.

System information (as much as possible)

https://gist.github.com/PhilETaylor/9d3c6591ed3d7d5273626f8c5d7ae193

Additional comments

Tested on FRONT and BACK end

avatar PhilETaylor PhilETaylor - open - 27 Mar 2017
avatar joomla-cms-bot joomla-cms-bot - change - 27 Mar 2017
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - labeled - 27 Mar 2017
avatar AlexRed
AlexRed - comment - 27 Mar 2017

I can confirm the problem


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/14935.

avatar Bakual
Bakual - comment - 28 Mar 2017

It's actually not related to com_fields, I can reproduce it in my extension (SermonSpeaker) where I use two editors in the same view.
So it's an issue with the editor (plugins) itself somewhere.
It works fine in 3.6.5.

@dgt41 I think you did some changes to how editors and their plugins work? Do you have a clue where that comes from?

avatar PhilETaylor
PhilETaylor - comment - 28 Mar 2017

It's actually not related to com_fields,

com_fields is being shouted about as the next major feature of Joomla.

The user experience of using com_fields is pathetic at best.

Therefore when Joomla starts shouting about its brand new com_fields type feature - the user experience will be crap

Then com_fields will get a bad name

ergo, this is a com_fields issue...

The fact that you have had the issue for (years?) in your extension but no one noticed, and yet within 5 mins of using com_fields I found this bug goes to show that this bug will be uncovered quickly by those attempting to use the new feature of Joomla...

I agree that the CODE that powers this feature is probably not in com_fields, however this IS used by the new (and some have said, exciting) new feature of Joomla - thus if you want to shout about great new features, at least make sure that all dependancies of those new features look and work well.

The new com_fields exposes much more functionality to the end user and super admins than before - and is uncovering a LOT of ancient bugs that need fixing before you can shout about how great a feature custom fields is.

Edit: You = Anonymous royal "you"

avatar Bakual
Bakual - comment - 28 Mar 2017

My statement was meant as a help as to where to look for the issue. Not that someone starts looking in com_fields to solve it because it is unrelated to it.
Of course, com_fields made it surface and can be used to test it easily (or you can install my SermonSpeaker component and look at the "Speaker" edit view).

The fact that you have had the issue for (years?) in your extension

As I wrote it works fine in 3.6.5 but is broken in staging. It's likely related to some changes in how the editors and their plugins work.

avatar dgt41
dgt41 - comment - 28 Mar 2017

@PhilETaylor @Bakual I'm afraid there is no way to go around this in 3.x due to B/C. Let me explain:
All XTD Buttons in 3.x are using the same function (declared by each editor, that's the problem) to interact with the editor namely jInsertEditorText(tag, editor);
This means, due to the way javascript overrides functions, the last registered editor will be the one that will have the above function coupled with it's own code.
This problem exist for many years but it's very obvious with com_fields. The solution is already merged:
#12561

BUT AGAIN: due to B/C promise the new functionality cannot be used as a replacement to the old one (please read the code before making assumptions here).

avatar PhilETaylor
PhilETaylor - comment - 28 Mar 2017

So this is a "wont fix" in 3.x even though a solution has been merged?

Just another "feature" of com_fields that brings down the whole quality of the new "flagship feature" :-(

avatar dgt41
dgt41 - comment - 28 Mar 2017

PS: The core buttons should work with different editors, tho, if not, there's a bug

@PhilETaylor core buttons should work, if not, there's a bug in my code!

Check these lines: https://github.com/joomla/joomla-cms/blob/staging/media/com_content/js/admin-article-pagebreak.js#L27-L32

avatar PhilETaylor
PhilETaylor - comment - 28 Mar 2017

The buttons I tested are shown in the image in the opening post of this issue.

Those are the features that are currently entering their results into the wrong editor.

Those are core Joomla features, installed from scratch with joomla-cms/staging@1f89fb09d941d3c7d14b366ffe84d880f9347cf7 code.

I have no custom "anything" installed.

So are you now saying that this is a bug that could and should be fixed in 3.x?

avatar dgt41
dgt41 - comment - 28 Mar 2017

@PhilETaylor make sure that you are testing with #14902

avatar dgt41
dgt41 - comment - 28 Mar 2017

So are you now saying that this is a bug that could and should be fixed in 3.x?

Maybe you got me wrong: the solution is not universal, meaning core buttons and core editors will work as they were intended, everything else won't.

I hope this clears things up

avatar PhilETaylor
PhilETaylor - comment - 28 Mar 2017

I hope this clears things up

Nope.

Are you saying that the core editors, when loaded in the custom fields, the core buttons in the opening post of this issue (Read More, Article) SHOULD work if multiple editors are on the page? - They Dont, hence this issue being opened

I have told you what I am testing with in my opening post, and again 14 mins ago, the revision used was 1f89fb0 which is 3 revisions after the merging of your code in commit 2dcfcd8

avatar Bakual
Bakual - comment - 28 Mar 2017

@dgt41 Then there is a bug in your code because the core buttons don't work that way ?
They insert always into the first editor on the page and the modal doesn't close.
Doesn't matter if it is a custom field or one defined in the form.xml.

avatar dgt41
dgt41 - comment - 28 Mar 2017

@PhilETaylor I'm an idiot, I should carefully read the descriptions ?
So you are right the fields button sucks big time as it's using only the olde code
https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_fields/views/fields/tmpl/modal.php#L29-L37

compare it to https://github.com/joomla/joomla-cms/blob/staging/media/com_content/js/admin-article-pagebreak.js#L27-L32

and you have your solution.

PS Please don't use inline scripts!

avatar PhilETaylor
PhilETaylor - comment - 28 Mar 2017

Video taken on e5f6dcf (This is the very latest HEAD of staging at the time of writing)

http://screenshot.myjoomla.io/2f2g251m2i0f

avatar dgt41
dgt41 - comment - 28 Mar 2017

@Bakual is that tinyMCE, or none, codemirror?
tinyMCE got a lot of code changed...

avatar dgt41
dgt41 - comment - 28 Mar 2017

@PhilETaylor can you try with editor none or codemirror to figure out if this is the buttons or the editor code that misbehaves?

avatar Bakual
Bakual - comment - 28 Mar 2017

@dgt41 TinyMCE in current staging. Tested using the Module and Menu buttons.
It works indeed with CodeMirror. So seems to be related to Tiny.

avatar PhilETaylor
PhilETaylor - comment - 28 Mar 2017

@dgt41 Those buttons dont appear (on the fields tab) when you select NONE as an editor. (Another bug!)

When you select CodeMirror you only get the buttons under the Content Editor on the Content Tab:
screen shot 2017-03-28 at 14 50 51

The custom buttons dont load under editors (CodeMirror) on the Fields tab... (another bug?)

screen shot 2017-03-28 at 14 51 07

avatar dgt41
dgt41 - comment - 28 Mar 2017

@PhilETaylor @Bakual there was a problem with the button configuration, I proposed #14417
but @Fedik come up with another idea, so the bug must be somewhere there

avatar Bakual
Bakual - comment - 28 Mar 2017

So you are right the fields button sucks big time as it's using only the olde code

Umm, those buttons work fine in Codemirror as well, same as all other buttons do. And they break with the second instance of TinyMCE as all other buttons do.
The code for them was based on the one for the modules button, so it can't be that it sucks that "big time".

avatar dgt41
dgt41 - comment - 28 Mar 2017

@Bakual

fieldIns = function(id) {
	window.parent.jInsertEditorText("{field " + id + "}", "' . $editor . '");
	window.parent.jModalClose();
};
fieldgroupIns = function(id) {
	window.parent.jInsertEditorText("{fieldgroup " + id + "}", "' . $editor . '");
	window.parent.jModalClose();
};

There is no way this code to ever work with multiple instances of editors
should be something like:

		/** Use the API, if editor supports it **/
		if (window.parent.Joomla && window.parent.Joomla.editors && window.parent.Joomla.editors.instances && window.parent.Joomla.editors.instances.hasOwnProperty(editor)) {
			window.parent.Joomla.editors.instances[editor].replaceSelection(tag)
		} else {
			window.parent.jInsertEditorText(tag, editor);
		}
avatar Bakual
Bakual - comment - 28 Mar 2017

There is no way this code to ever work with multiple instances of editors

Then I wonder why it works fine with multiple instances of Codemirror when the editor is specified in the form xml.

Honestly, if that code wouldn't work, then we have a problem with 3.7.0 as there would be a B/C incompatible change. This code HAS to work as it is likely present in many plugins.

avatar dgt41
dgt41 - comment - 28 Mar 2017

@Bakual so jInsertEditorText() is a function that every editor declares. Due to nature of javascript and to chained calls only the last one will be available. If you have a solution for this please make a PR an revert my approach of multiple instantiation for the editors...

avatar Bakual
Bakual - comment - 28 Mar 2017

I have no clue about JavaScript. So I can neither fix nor revert it myself as I wouldn't know what code to change.
But I know what our B/C promise says: If we apply changes that break existing functionality, they have to be reverted and delayed to 4.0.

But as I wrote, it works with two instances of CodeMirror. It just doesn't work for TinyMCE for some reason.

avatar dgt41
dgt41 - comment - 28 Mar 2017

@Bakual then either:

  • take out the editor field from com_fields
  • append automatically the same editor as the one that user is using (or the default from configuration)

You can have multiple instances of different editors in 3.7 as this was never actually working till my PR, which unfortunately is introducing a new method which is not B/C with the crap the existed for a decade...

Pick wisely

avatar AlexRed
AlexRed - comment - 28 Mar 2017

for example the extension Jdownload use multiple editor and in Joomla 3.6.5 is ok to use the editor plugin "Module" in the second editor instances and the modulo code is in the second editor content. But in Joomla 3.7.0stagin all the editor plugin go to the first editor instances.
(sorry for my bad english)

avatar PhilETaylor
PhilETaylor - comment - 28 Mar 2017

take out the editor field from com_fields

Lol - another "Headline feature" being removed rather than implemented correctly is not the way forward really.

avatar Bakual
Bakual - comment - 28 Mar 2017

You cannot have multiple instances of different editors in 3.7 as this was never actually working till my PR, which unfortunately is introducing a new method which is not B/C with that crap jInsertEditorText() the existed for a decade...

I think we misunderstood eachother then here. I was talking about two instances of the same editor. Not mixed ones. So two TinyMCE or two Codemirrors are both working fine in 3.6.5. Two Codemirrors still work fine in staging, but two TinyMCE are broken.
Exactly as @AlexRed confirms as well.

  • take out the editor field from com_fields
  • append automatically the same editor as the one that user is using (or the default from configuration)

And again, I'm not talking about com_fields, it's broken also when two editors are specified in the form.xml (like in my extension SermonSpeaker or JDownloads).
The custom editor field doesn't allow to specify the editor type anyway. It will use the default one (either user setting or global). So both options wouldn't solve the issue and don't make any sense ?

avatar dgt41
dgt41 - comment - 28 Mar 2017

I guess there is a bug somewhere in #14520

avatar Bakual
Bakual - comment - 28 Mar 2017

That one isn't even merged yet and it isn't about the button configuration. The buttons are there and work, just the JS from the modal has the wrong target and the modals don't close.

avatar dgt41
dgt41 - comment - 28 Mar 2017

I just realised that, that PR is not merged!

@PhilETaylor please test

avatar dgt41
dgt41 - comment - 28 Mar 2017

@Bakual nope, that's the problem, buttons misbehaving due to bad configuration (js side)

avatar Bakual Bakual - change - 29 Mar 2017
Status New Closed
Closed_Date 0000-00-00 00:00:00 2017-03-29 14:20:01
Closed_By Bakual
avatar Bakual Bakual - close - 29 Mar 2017
avatar Bakual
Bakual - comment - 29 Mar 2017

@dgt41 You're right, that did fix it for me both for custom fields and when specified in the XML.
I'm closing this issue since there is a PR.

avatar PhilETaylor
PhilETaylor - comment - 29 Mar 2017

PLEASE watch this video http://screenshot.myjoomla.io/232g3O1C0K0B

It seems many of you in this thread misunderstand the reported issue.

#14520 DOESNT fix the reported issue.

avatar zero-24 zero-24 - change - 29 Mar 2017
Status Closed New
Closed_Date 2017-03-29 14:20:01
Closed_By Bakual
avatar zero-24 zero-24 - reopen - 29 Mar 2017
avatar zero-24
zero-24 - comment - 29 Mar 2017

Thanks reopening.

avatar dgt41
dgt41 - comment - 29 Mar 2017

@PhilETaylor out of curiosity, what are the names of the fields in your test?

avatar brianteeman
brianteeman - comment - 29 Mar 2017

@dgt41 doubt that matters as I can replicate it

avatar dgt41
dgt41 - comment - 29 Mar 2017

@brianteeman the reason that I'm asking this is because the solution in #14520 is based on the names of the fields...

avatar PhilETaylor
PhilETaylor - comment - 29 Mar 2017

for the record then, my custom fields editors were named

  • editor1
  • editor2

However, if you are expecting things to work based on user inputted field names, then we have lost already and we should all just pack up and go home. :-)

I have tested again and the custom field names make no difference to the issue reported.

avatar Bakual
Bakual - comment - 29 Mar 2017

@PhilETaylor That's exactly the issue I was talking about as well and which got fixed for me. After clearing browser cache of course since the JS has changed.
The only difference I see is that I named my field just "editor" and I only used one custom editor field.

I can try again later this evening when kids are sleeping.

avatar PhilETaylor
PhilETaylor - comment - 29 Mar 2017

Im now home on a different mac.

This is not a cached issue. This is not fixed by #14520 with caching disabled.

This happens with even a single custom editor field called "editor" - the results of the buttons in the bottom row are injected to the wrong editor.

tested just now on the very latest 4ba67b7

avatar PhilETaylor
PhilETaylor - comment - 29 Mar 2017

If interested here is another multiple editors on page bug reported just now #14990

avatar Bakual
Bakual - comment - 29 Mar 2017

@PhilETaylor Can you try what happens if you set "Show Buttons" to "Yes" in the editor field plugin? Looks like that setting doesn't work properly with TinyMCE. By default ("Use Plugin") it would be off which would mean no buttons at all, but TincyMCE shows them still but broken.
Codemirror properly follows that setting.

So there is another bug in Tiny which needs fixing, but not sure how much related it is.

avatar PhilETaylor
PhilETaylor - comment - 29 Mar 2017

So, testing with 4ba67b7, with #14520 applied, an editor called editor, AND with setting "show buttons" to Yes (as opposed to 'use from plugin' default), a clean cache, I CAN STILL REPLICATE THE REPORTED ISSUE! (this issue, and also #14990)

avatar PhilETaylor
PhilETaylor - comment - 29 Mar 2017

I can confirm that with CodeMirror enabled (instead of TinyMCE) this issue doesn't happen. However thats probably because the buttons for the code mirror editor are those under the editor - and for tinymce they are integrated into the tinymce interface.

avatar Bakual
Bakual - comment - 29 Mar 2017

That's strange. I really wonder where the difference is between your installation and mine.

avatar PhilETaylor
PhilETaylor - comment - 29 Mar 2017

As my videos clearly show - Im resetting hard my git, installing from scratch, and then replicating the issues....

avatar Bakual
Bakual - comment - 29 Mar 2017

My installation is pretty much the same. It's an installation only a few days old with sample testing data and updated to current staging.
I can install it fresh this evening but I honestly doubt this will make a difference.

avatar dgt41
dgt41 - comment - 29 Mar 2017

@PhilETaylor try saving the tinyMCE plugin!

avatar PhilETaylor
PhilETaylor - comment - 29 Mar 2017

by "tinyMCE plugin" I assume you mean Extensions -> Plugins -> Editor - TinyMCE

Testing with 4ba67b7, with #14520 applied,

I did

  • Extensions -> Plugins -> Editor - TinyMCE -> save
  • Hard clear google chrome cache
  • Open Google Chrome Inspector, Netwrok Tab, Check Disable Cache

No difference - same issue as reporting in the opening comment of this issue (Pagebreak issue #14990 is also not fixed by this test)

avatar PhilETaylor
PhilETaylor - comment - 29 Mar 2017

hmmm @mbabker what was that issue with the Patch tester where it doesnt apply patches right?

avatar PhilETaylor
PhilETaylor - comment - 29 Mar 2017
	modified:   administrator/components/com_content/models/forms/article.xml
	modified:   media/editors/tinymce/js/tinymce.js
	modified:   media/editors/tinymce/js/tinymce.min.js
	modified:   plugins/editors/tinymce/tinymce.php

These are my file modifications after applying #14520 with PAtch Tester

avatar mbabker
mbabker - comment - 29 Mar 2017

It doesn't patch files. It replaces files based on the contents of the remote branch. So if that branch is nowhere close to matching staging, that would cause side effects too.

avatar Fedik
Fedik - comment - 29 Mar 2017

no need to save, no need to reinstall ?
I can confirm the bug also, and I think I know the reason, but I will tell nothing :neckbeard:

well, if I get some free time I will make a fix

@Bakual try to add next fields in article XML, and you will get the same bug ?

<field name="editor1" type="editor" 
    label="Text2" description="" filter="JComponentHelper::filterText"  />

<field name="editor2" type="editor" 
    label="Text3" description="" filter="JComponentHelper::filterText" />
avatar dgt41
dgt41 - comment - 29 Mar 2017

@Fedik but you already told us ?

avatar Bakual
Bakual - comment - 29 Mar 2017

@Fedik Ah I see, if "hide" isn't specified, then the bug is still present.
My component has that specified and that's why it was fixed for me.
I also had one button set to hide (due to doing testing) in my editor field which is the difference to the setup of @PhilETaylor.

avatar PhilETaylor
PhilETaylor - comment - 30 Mar 2017

ok I have manually applied every change in #14520 to f45c0da and I can still replicate the reported issue

Im 100% convinced #14520 doesnt resolve this reported issue

avatar Bakual
Bakual - comment - 30 Mar 2017

Closing this again since #14520 now fixes the issue for real ?

avatar Bakual Bakual - change - 30 Mar 2017
Status New Closed
Closed_Date 0000-00-00 00:00:00 2017-03-30 18:27:54
Closed_By Bakual
avatar Bakual Bakual - close - 30 Mar 2017
avatar PhilETaylor PhilETaylor - comment - 30 Mar 2017

Add a Comment

Login with GitHub to post a comment