Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Link-dropdown breaks in TinyMCE when other EditorConfig is used on one of the Elements #1120

Closed
4 tasks done
baukezwaan opened this issue Nov 21, 2023 · 7 comments
Closed
4 tasks done

Comments

@baukezwaan
Copy link

baukezwaan commented Nov 21, 2023

Affected Version

This bug exists in ^4 and ^5 release lines.
I've tested it against 5.1.0 and 4.11.0

Description

When using multiple TinyMCEConfig-configurations, for example to have a small editor and a full editor, the sslink dropdown stops working. This occurs when you have 2 (or more) different elements on one page, where one of them is using an own TinyMCEConfig-configuration. The link-functionality stops working. Only the first Element that was opened, will have a working link in the editor.

tinymce-bug.mp4

Steps to Reproduce

  1. Install elemental in SS5 or SS4 (via composer.json)
        "silverstripe/recipe-cms": "^4",
        "dnadesign/silverstripe-elemental": "*"

or

        "silverstripe/recipe-cms": "^5",
        "dnadesign/silverstripe-elemental": "*"
  1. Add Elemental-area to alle pages (via elemental.yml)
Page:
  extensions:
    - DNADesign\Elemental\Extensions\ElementalPageExtension
  1. Create a new TinyMCE editor config (via _config.php)
    $oSmallEditor = clone TinyMCEConfig::get('cms');
    $oSmallEditor->setButtonsForLine(1, []);  // not  neccecary, but makes easier to see the diffrence between editors
    HTMLEditorConfig::set_config('cms_small', $oSmallEditor);
  1. Create a new Element with a diffrent tinymce-config
class TextImageElement extends BaseElement
    {
        private static $table_name = 'Elemental_TextImage';
        private static $singular_name = 'Text Alternate';
        private static $icon = 'font-icon-block-promo-3';

        private static $db = [
            'ContentFirst'  => 'HTMLText',
        ];

        public function getCMSFields()
        {
            $oFields = parent::getCMSFields();

            $oFields->addFieldsToTab('Root.Main', [
                    HTMLEditorField::create('ContentFirst', 'Area with cms_small-config')->setEditorConfig('cms_small'),
                ]
            );

            return $oFields;
        }
    }
  1. Add two diffrent elements to a page
  2. Observe the link-dropdown (both in the right-click context menu, and in the "insert link" button) is broken on the second element you open

Installer CI run

Some of these PRs won't be green until others are merged.
This is a CI run in silverstripe/installer showing that together they go green.
https://github.com/creative-commoners/silverstripe-installer/actions/runs/7634747838

Acceptance Criteria

  • Bug is fixed in 1.13
  • Contraints are updated in asset-admin + cms to set minimum patch version of admin with fix
  • Admin is manually merged up, new 5.1.x tag is automatically tagged
  • asset-admin and cms are manually merged-up with correct patch constraint for admin set during merge up

PRs

NOTE after the admin PR is merged, reassign to Guy to set the correct constraint for the next two

cross-major mergeup commits to validate

@emteknetnz
Copy link
Member

emteknetnz commented Dec 7, 2023

I've done some initial investigation in to this, I can replicate this in CMS 4.13 and 5.1 though it looks very hard to fix this.

  • HtmlEditorField.js (react component, not entwine) will load a script tag with a src pointing to a generated tinymce file in public/assets/_tinymce when you open an element
  • Different files are created for different configs e.g. tinymce-cms-07deb9ef54.js for default config or tinymce-cms_small-6d3061b8d8.js for custom config
  • These files are generated by TinyMCECombinedGenerator.php
  • The generated files define window.tinymce (and window.tinyMCE) - in the minified code it should look like window.tinymce=rE
  • There is logic in HtmlEditorField.js to only create a script tag if window.tinymce is undefined
  • This means that whatever element you open first will have its tinymce script loaded and the 'link' dropdown will work for that element, but not the other element that uses a different config
  • If you modify the javascript so that you loaded the script regardless if window.tinymce is defined or not, and you comment out the if (window.tinymce) { return } js statement in TinyMCECombinedGenerator.php, things won't magically work as window.tinymce will just get defined again.

Seems like the existing system simply isn't designed to have multiple tinymce configs loaded at the same time.

@emteknetnz emteknetnz removed their assignment Dec 7, 2023
@baukezwaan
Copy link
Author

Thanks for looking into it @emteknetnz !
The conclusion is fine, apparently this combination is not working for Elemental. Although limiting the buttons on the editor is user-friendly, it is not a major issue.

Because it is such a strange behavior, it took us a while to find the cause. So hopefully this issue will at least help future visitors identify and solve this problem (tldr: don't use multiple tinymce configs in combination with elemental).

It's probably wiser to spend time on more common issues. Should we close this issue, or dou you want to leave it open?

@emteknetnz
Copy link
Member

Just leave it open for now

@sig-peggy
Copy link

For others hitting this issue, I've found 2 slightly hacky work-arounds. Either:

  1. Set the elemental block to private static $inline_editable = false; so that it has to edit on it's own screen, or
  2. On the relevant Page class, add a hidden HTMLEditorField with the same config that you want your block(s) to use - this will create the required config file which inline-editable blocks can then use without having to generate their own.

@LABCAT
Copy link

LABCAT commented Dec 19, 2023

I'm also experiencing this issue when the page has HTMLEditor fields that use a different config (eg for an intro field).

This bug seems quite similar to this one:
silverstripe/silverstripe-admin#1512

Perhaps the PR for that has some hints for how to fix this.

@emteknetnz
Copy link
Member

emteknetnz commented Jan 24, 2024

@GuySartorelli I've added some AC's re merge-ups and contraints to follow - just update the constraints in the manual merge-up and ping me so I can double check them as a form of peer review

@emteknetnz
Copy link
Member

cross-major mergeup commits to validate added to description looks good

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

No branches or pull requests

5 participants