Fixes 2814, 2815

[simondate]

Fixes -

• WYSIWYG editor stripping out html #2814
• CKEditor toolbar element 'RemoveFormat' not loaded in #2815

Two issues with CKEditor.

[simondate]

Just updated this PR as it was still stripping the aria-hidden="true" attribute.

Note for testers - You'll need to update the conf/config.json file, to include this -

 "ckEditorHtmlSupport": {
    "allow": [
      {
        "name": "^(blockquote|ul|ol|span)$",
        "attributes": true,
        "classes": true,
        "styles": true
      },
      {
        "name": ".*",
        "classes": true,
        "styles": true
      },
      {
        "name": "^a$",
        "attributes": {
          "target": true
        }
      }
    ],
    "allowEmpty": [
      "span", 
      "i"
    ]
  },

[simondate]

Just discovered that the any links to Glossary objects also weren't working.

The latest commit fixes this too.

[oliverfoster]

Won't this allow onclick attributes on all of the aforementioned tags? Which means that one AAT user could add JavaScript that another user could execute?

[StuartNicholls]

This was added to allow aria-hidden, if we think the above is a risk then we can add specific attributes. I believe like this:

attributes: { foo: 'true', bar: true }

[oliverfoster]

It definitely is a risk if it allows all attributes.

[StuartNicholls]

@simondate , so allow, aria-hidden, aria-label?

[simondate]

I would want data-glossaryterm included as it's the way you open glossary items within elements on the page.

If onclick is the only thing we're concerned about could we just disable that?

[oliverfoster]

Does this work?

      {
        "name": ".*",
        "attributes": "^(id|role|aria-.+|data-.+|target|href)$",
        "classes": true,
        "styles": true
      },

In theory that should allow all aria- and data- prefixed attributes as well as role and id.

[oliverfoster]

bump

[simondate]

I've been using this PR for months with my own clients, no issue.

I just thought that we weren't doing any more merges to the "Legacy" AAT anymore?

[oliverfoster]

It's been waiting for you to respond to the above question. It has security implications.

[simondate]

We might also need target so that links can open in a new tab/window.

@StuartNicholls / @fosterc1 can you think of anything else you would want?

[mrblonde77]

Any update on this topic? We switched to the current Authoring Tool and Framework Version, and now we have problems with tables in the ckeditor. I assume that it is related to this problem: CKEditor toolbar element 'RemoveFormat' not loaded" #2815