LGTM

Sorry, the fix is not a fix. And should not be acepted.
You break overide posibilities for TinyMCE options, in subform, and some more stuff I not remember.
Also it will not work for newly added row in subform.

Editor-XTD buttons broken because they hardly linked to Editor ID.
There need api changes, to make the EXT button access to Active editor (lastest editor in focus), but that need some more work, and probably some b/c break.

Editor-XTD buttons broken because they hardly linked to Editor ID.

True, they only get the editor name as their input, probably we could pass the id as well:

There need api changes, to make the EXT button access to Active editor (lastest editor in focus), but that need some more work, and probably some b/c break.

So basically a store that each editor needs to subscribe and update the state each time it takes focus but I don't really follow you on the API part for the buttons, how would that work? Just ask the latest in focus and then apply the command on that editor?
I really would like someone with strong PHP skills to scrap the complete Editor stack and come up with something that truly works with multiple instances, we keep patching this ugly thing since 3.6 and each time someone comes back with more failing cases...

Just ask the latest in focus and then apply the command on that editor?

Yeap, kind of that.

The idea like this:
At any point of time on the page can be only One active editor. So we can use it.
When User interact with edittor container the editor script should mark this editor instance as active, (I mean interaction with container with editor and all ext-button, basycaly parent wrapper.)
Kind of: Joomla.editor.setActive(editor). As part our Editor API.
Then button just do: Joomla.Editor.getAtive().doSomething()

Basically (pseudo):

// Only define editors if not defined
window.Joomla.editors = window.Joomla.editors || {};

// Dumb last focused editor state
window.Joomla.editors._active = null;
window.Joomla.editors.setActive = (val) => window.Joomla.editors._active = val;
window.Joomla.editors.getActive = () => window.Joomla.editors._active;

// An object to hold each editor instance on page, only define if not defined.
window.Joomla.editors.instances = window.Joomla.editors.instances || { };

Yeap kind of that.

Then

Joomla.editors.instances['jform_articletext'].replaceSelection('Joomla! rocks')

will become

Joomla.editors.getActive().replaceSelection('Joomla! rocks')

Also in setActive() can store only instance key, and then use this key in getActive():

getActive = () => Joomla.editors.instances[Joomla.editors._active];

But that a details, can be anything.

Joomla.editors.getActive().replaceSelection('Joomla! rocks')

Can we use proxy for the state (eg https://github.com/windmaomao/proxy-state) and make it promise based? Or am I complicating it too much?

I do not have much expiriance with Proxy. Sounds complicated to me :)
Can be Promise, well, anything that work

I do not have much expiriance with Proxy. Sounds complicated to me :)

It's not ultra weird, this example from MDN is probably closer to what we need: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy#validation

Hmhm, I would do without, and look how it will go.

We already have similar (synchronous) code:

We already have similar (synchronous) code:

FFS that code is wrong ?

  setCurrent: (element) => { 
     window.Joomla.Modal.current = element; 
   }, 
   getCurrent: () => window.Joomla.Modal.current, 

If it work, then nothing wrong hehe

@Fedik nevertheless here's a PR #39450

@Fedik out of curiosity #39344 shouldn't have the same problem, the buttons are not bound to the editor id iirc

shouldn't have the same problem, the buttons are not bound to the editor id iirc

That modal will not have the problem, it just an modal, it does nothing with editor.
You registre "click", then onClick create modal on fly, and use it. So you always have a reference to it.

The media/user fields, also should work in theory. But I still would change them to use "events" instead of callbacks. But that another topic.

That modal will not have the problem, it just an modal, it does nothing with editor.

I realised that the problem in in the url of the iframe the &editor='. $$name . '

You registre "click", then onClick create modal on fly, and use it. So you always have a reference to it.

I think we should just create a simple API that collects the data for the button (text, icon, etc) pass them to joomlaOptions and have the editor create them and bound them per instance. Should be less problematic going forward

I realised that the problem in in the url of the iframe the &editor='. $$name . '

Yeah. As long as the Input id is correct, this should continue to work.
This is a problem of "select" field, not modal itself. This a reference to Input for callback inside iframe.

I think we should just create a simple API that collects the data for the button (text, icon, etc) pass them to joomlaOptions and have the editor create them and bound them per instance.

That could work. But that should be compatible with any editor, not only tinymce.

But that should be compatible with any editor, not only tinymce.

We will implement it for tinyMCE, Codemirror and None, so plenty examples for anyone to pickup the code for their own implementation. I think we should evaluate which option will be also easier for maintenance but also for 3rd PD. I don't have all the answers, I'm just shouting ideas here

Yeah, need to make some concept first.
Could be that each button should own script, wich on click just do
Joomla.editors.getActive().replaceSelection('Joomla! rocks').

Probably some simple button can have a callback, and more complex a script, or callback that run code from script :)
Need to think.

I don't have all the answers

Me either :)

I realised that the problem in in the url of the iframe the &editor='. $$name . '

Yes glad you got there in the end. But the reason for that is that the buttons currently reference the last part of the name so jform[com_fields][subform][row0][field1] and jform[com_fields][subform][row1][field1] both have a name in the javascript options of field1 and only have modals loaded for the first row because afterwards https://github.com/joomla/joomla-cms/blob/4.2-dev/plugins/editors/tinymce/src/PluginTraits/DisplayTrait.php#L120 returns false and therefore doesn't load the modals.

Also it's not just modals for example readmore which doesn't have a modal but instead has a click event uses "click":"insertReadmore('jform_com_fields__subform__row0__field1');return false;", As it uses the ID rather than the editor name...

I mean I put in some code that although it is a b/c break works ;) Admittedly I've broken the initial load of tinymce even more - but pretty sure i can fix that in another 10 minutes - it's just the fact the default field misses xtdButtons altogether. But if you guys have a counter proposal happy to figure that out. But right now I'm not sure you do?

You break overide posibilities for TinyMCE options, in subform, and some more stuff I not remember.

Also going to need some extra explanations here....

Also needs fixing that all tiny xtd buttons for a subform field editor have the same id due to https://github.com/joomla/joomla-cms/blob/4.2-dev/plugins/editors/tinymce/src/PluginTraits/XTDButtons.php#L61

Yes glad you got there in the end. But the reason for that is that the buttons currently reference the last part of the name so jform[com_fields][subform][row0][field1] and jform[com_fields][subform][row1][field1] both have a name in the javascript options of field1 and only have modals loaded for the first row because afterwards

Theoretically speaking #39351 should be orders of magnitude easier to patch for subforms. A possible way out of this rabbit hole might be a twisted code of the xtd-buttons:

• have a template element with the basic structure of a modal
• Each editor, on initialisation (here's were you cater for copies on a subform) should create the buttons and the modals client side (this should be the same as the SSR)
• The advantage here is that you can bound the url/modal/whatevr to the actual (from the DOM) id of the editor which should solve the issue
• Problem: the CMS doesn't have any method to enqueue some HTML fraction, so either use a unique class and ignore the multiple instances of the template (the template would be created by the HTMLHelper::_('bootstrap.modal') using an extra param) or figure out a enqueue method like #37330)

But all these are hacks around an API that was never designed to support multiple instances of any editor per page...

I mean the specific code dates back to this #14520

But I don't understand why we've done this per part of field name and not just used the full name or id? I assume I'm missing something obvious?

But all these are hacks around an API that was never designed to support multiple instances of any editor per page...

Depends who you want responsible for what :D PHP these days just assembles all the form field params together. The JS is doing all the rendering these days as you know. So honestly I'm not totally sure where the "root problem" is other than it's how the options are being passed through.

I agree about templating and thought about bodging it. But even that involves tiny being aware it's being used in a subfield which isn't how it should be in theory. It should somehow be hooking into https://github.com/joomla/joomla-cms/blob/4.2-dev/build/media_source/system/js/fields/joomla-field-subform.w-c.es6.js#L253 to do the id replacement (a new event??) although subform-row-add might be good enough. But making that templatable is also a huge b/c break in it's own right and would likely be tiny specific anyhow :(

It should somehow be hooking into

Ok, some insight in the way this whole think works maybe will show why I'm leaning into a client side rendering solution. So when php sends the html (assuming that the subform is null at this point) the tinymce js gets all the .js-editor-tinymce elements and apply the init method. At that point all the modals are pre-rendered from the php and the Bootstrap js just binds them to the js and the whole thing works as expected. When a user presses the + button to add a new editor field the subform js clones the .js-editor-tinymce element (that's without the modals as they are one node above, first problem) and on the fly adjusts the ids, etc and THEN the tinymce init retriggers on that element.
You might recall that I was calling that all the interactive elements should be done as custom-elements (they have a proper lifecycle which is extremely helpful for subforms) but the main problem here is that the modals (or any std-button functionality) basically is disconnected from the instance of the cloned editor (same thing applies for the other editors).
Either these should communicate with some bus-sub event system as @Fedik is proposing or another system that allows the buttons to be bound to a particular instance. BTW when was the nested subform introduced? It seem that it was rushed (I remember @Fedik writing code that wasn't exposed because there were cases like this that the current status didn't cope with)

But I don't understand why we've done this per part of field name and not just used the full name or id? I assume I'm missing something obvious?

Look: you have ID jform_com_fields__subform__row0__field1, you open the page, all works.
Then you add a new row in subform, now the editor have an id jform_com_fields__subform__row1__field1, therefore there no options for the editor with this ID in the options sorage. Your next move?

I mean I put in some code that although it is a b/c break works ;)

It looks like works, but it is not.
When you try multiple editors, when each have a diferent options, you will see that nothing works as expected.

@Fedik @wilsonge will this code fixes (the modal, for now, for the inline code we need to do a similar search replace) the problem?

      arr.forEach((xtdButton) => {
        const tmp = {};
        tmp.text = xtdButton.name;
        tmp.icon = xtdButton.icon;
        tmp.type = 'menuitem';

        if (xtdButton.iconSVG) {
          icons[tmp.icon] = xtdButton.iconSVG;
        }

        if (xtdButton.href) {
          const modal = element.parentNode.parentNode.querySelector(`#${xtdButton.id}_modal`);

          // Update the editor url parameter of the modal
          if (modal && modal.dataset.url) {
            const url = new URL(modal.dataset.url);
            url.searchParams.delete('editor');
            url.searchParams.append('editor', element.id);
            modal.dataset.url = url.toString();

            tmp.onAction = () => {
              modal.open();
            };
          }
        } else {
          tmp.onAction = () => {
            // eslint-disable-next-line no-new-func
            new Function(xtdButton.click)();
          };
        }

        buttonValues.push(tmp);
      });

Looks haky, but if it works then I am fine ?

Of course it's hacky. We seriously need to re architecture the Editor and buttons to fit the current multi editor status (since the introduction of Custom Fields) and also eliminate all the new Function...

I am closing it in favor of #40202. Please review and test ;)