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]

I wouldn't go with disabling things. Something new is always added that makes denies redundant. Permissive allows are generally a better idea (as per the French vs British legal system).

How about we don't set the default config to allow/deny extra things that are specific to a vendor, but we instead just add allowEmpty and RemoveFormat to support the given use-case?

[simondate]

allowEmpty would fix the original issue, but I feel the glossary extension should be supported, considering it's a Core extension

[oliverfoster]

Does this work?

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

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