w3c / w3c-website

W3C Website feedback and bug reports
https://www.w3.org/
244 stars 81 forks source link

Authors need more control on the allowed markup #104

Closed jean-gui closed 2 years ago

jean-gui commented 2 years ago

While being great to ensure a consistent styling across the site, the current component system is sometimes too limiting for what we would need to do as it is very restrictive on the kind of markup we can use. This issue is a meta to discuss solutions to this limitation.

On https://www.w3.org/blog/news/archives/9330 (and other news entries) there is a small picture. Displaying it full-width would look weird. #98 is a bit similar in that sense as even if the ratio was right, I don't know if we'd want the logo to take that much space. Another example is https://www.w3.org/blog/2020/03/css-x/ which has a couple embedded images. Now, we could use a 50/50 component for such images, provided that #94 was implemented, but I'm not sure it would be so great, as this component tends to break the flow of reading. It seems more targeted to creating a new section of content than embedding an image in text. Yet another example is https://www.w3.org/standards/webdesign/i18n which shows several images embedded within textual content and would look very odd in a full-width image or 50/50 component.

As a body that creates standards for the Web, W3C also has a need to showcase new technologies (such as vertical text in #100) and leverage existing ones such as internationalizing pieces of content (#95 or https://www.w3.org/International/questions/qa-navigation-select.en are good examples).

How can we ensure that we can enter all the markup that we need while still ensuring a general consistency of the site?

Related issues: #93 , #94 , #98 , #100 .

maries24 commented 2 years ago

Hi @jean-gui @vivienlacourba and @koalie (cc @simonrjones)

These issues can be addressed (some fully, some partially) by updating the configuration of the advanced content filtering feature of the CKEditor WYSIWYG.

Configuration of CKEditor is documented in this page of the documentation of the Craft CMS Among other things, that page explains how to extend the set of markup elements and attributes that the content authors can use in the WYSIWYG.

For more information, here is the full documentation from CKEditor on advanced content filtering: https://ckeditor.com/docs/ckeditor4/latest/guide/dev_acf.html https://ckeditor.com/docs/ckeditor4/latest/guide/dev_allowed_content_rules.html

We can also:

I would suggest I tackle all these issues (#93 , #94 , #95, #98 , #100 ) together next time I'm picking up the work.

Things I'd need to know (and am happy to discuss if need be):

Many thanks!

jean-gui commented 2 years ago

Vivien and I spent some time last week trying to configure CKEditor using the doc you wrote but it was unclear if we needed and how to rebuild the library.

Would you like to allow authors to use any markup and attributes or would still want to limit what's possible?

We went through all HTML elements listed on https://developer.mozilla.org/en-US/docs/Web/HTML/Element and we believe we should be rather exhaustive in terms of what to allow, to avoid frustrating editors and give them the flexibility our documents often require.

Considering how much we should allow, I wonder if we shouldn't disable what we really don't want to see (basically: script, on*, style) instead.

Also, isn't there an HTML purifier library that we would need to configure?

Here is what we think we should allow:

Regarding attributes, looking at https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes we should allow:

And additionally, some elements need their attributes allowed:

Which 'extra' elements would warrant adding a button for them to the WYSIWYG?

Extra buttons we think would be useful in the UI:

maries24 commented 2 years ago

Note: when addressing this, also update CMS manual to explain need to focus on element to get list of custom styles available for it in CKEditor. Test what happens to elements not allowed but included as content of a code block.

maries24 commented 2 years ago

Hi @jean-gui @vivienlacourba @koalie I started working on this. I worked out how to configure CKEditor but unfortunately there is a further HTML purifier that still wipes out some elements on saving content. Before I spend too much time on this, I've messaged the Craft team to help me with the configuration of the HTML purifier. Once they get back to me with a solution, I can finish the task.

maries24 commented 2 years ago

Hi @jean-gui @vivienlacourba @koalie

I have modified the configuration of CKEditor to allow the elements and attributes listed above.

I have also added the ability to add images via CKEditor in the text component.

Outstanding work (I have more time for these tomorrow and they are the object of separate task cards):

I just wanted to push what I have done so far so you can carry on with content entry whilst I am still addressing the above.

jean-gui commented 2 years ago

Hi @maries24 It seems that the class attribute is still being stripped from source.

maries24 commented 2 years ago

Hi @jean-gui Thanks for spotting this. I had made a mistake in the config syntax. It's sorted now.

vivienlacourba commented 2 years ago

Hi @maries24

This looks very good. Some feedback below.

Which 'extra' elements would warrant adding a button for them to the WYSIWYG?

Extra buttons we think would be useful in the UI:

abbr

There seems to be a CKEditor plugin for this, can it be added? https://ckeditor.com/docs/ckeditor4/latest/examples/abbr.html

(...) img, if the code can be tweaked to allow positioning them using classes that would have to be created

If I click on the Image button, then next to the URL field there is a "Browse server" button that enables to select a previously uploaded asset. This generates the following markup in CKEditor source view:

<img alt="" src="https://cdn-dev.w3.org/cms-uploads/CSS-logo1s.png#asset:12535:url" style="height:211px; width:225px" />

This URL does not seem to be handled correctly by the frontend app, which generates the following markup:

<div class="component component--text">
<h2 id="testing-the-ckeditor-image-button">Testing the CKEditor image button</h2>

<p><img alt="" src="{asset:12535:url||https://cdn-dev.w3.org/cms-uploads/CSS-logo1s.png}" style="height:211px; width:225px"></p>

</div>

And the corresponding rendered HTML: ckeditor-image-from-button

See:

setting an id (instead of anchors, if that's possible)

We think the actual anchor button is not useful for us, but we rely on ids a lot. Can this button be replaced by one that enables to set an id?

jean-gui commented 2 years ago

For the image issue, the weird looking is coming from Craft's GraphQL response:

{
  "typeHandle": "textComponent",
  "contentField": "<h2>Testing the CKEditor image button</h2>\r\n\r\n<p><img alt=\"\" src=\"{asset:12535:url||https://cdn-dev.w3.org/cms-uploads/CSS-logo1s.png}\" style=\"height:211px; width:225px\" /></p>\r\n"
}

It looks like if the image URL has a fragment (asset:12535:url in this case) then it's being transformed to {the_fragment||the_url}. I don't know why it does that, but if you remove this fragment from the URL, it then seems to work fine.

@maries24 Is that something we should bring to Craft developers' attention? I've asked them and that's a bug they are fixing.

maries24 commented 2 years ago

Hi @vivienlacourba and @jean-gui

I have added the 'abbr' plugin to CKEditor. The Craft team issued a fix for the URL issue last Friday. I updated the plugin and deployed today.

Unfortunately, despite several feature requests for an 'add ID' button or plugin, there doesn't seem to be any. Using the 'Anchor' feature of the link plugin as a starting point for a custom plugin to add an ID is possible but not straightforward. The Anchor plugin creates a new 'a' element with an id and name attribute. The requirement here is for a plugin that identifies the selected element (be it an 'a', 'span', 'h2',...) and add an ID to it. That makes a significant difference in terms of implementation.

I would recommend creating a new issue for that and re-consider it next time we prioritise the list of issues.

vivienlacourba commented 2 years ago

Hi @vivienlacourba and @jean-gui

I have added the 'abbr' plugin to CKEditor. The Craft team issued a fix for the URL issue last Friday. I updated the plugin and deployed today.

Unfortunately, despite several feature requests for an 'add ID' button or plugin, there doesn't seem to be any. Using the 'Anchor' feature of the link plugin as a starting point for a custom plugin to add an ID is possible but not straightforward. The Anchor plugin creates a new 'a' element with an id and name attribute. The requirement here is for a plugin that identifies the selected element (be it an 'a', 'span', 'h2',...) and add an ID to it. That makes a significant difference in terms of implementation.

I would recommend creating a new issue for that and re-consider it next time we prioritise the list of issues.

Done see: https://github.com/w3c/w3c-website/issues/133