instructure / canvas-lms

The open LMS by Instructure, Inc.
https://github.com/instructure/canvas-lms/wiki
GNU Affero General Public License v3.0
5.66k stars 2.5k forks source link

Broken HTML details and summary disclosure elements in Rich Text and Published versions #1502

Open reubenlillie opened 5 years ago

reubenlillie commented 5 years ago

Summary:

The HTML editor permits valid use of <details> and <summary> elements. However, the Rich Text editor does not render a working disclosure widget, and published versions of Pages do not preserve the markup at all.

Nesting content inside <details> and <summary> elements might otherwise feel like a feature request. But since this is valid HTML since 4.11.1, recognized by the editor, and markup is being sanitized or escaped at some point, it is properly a bug. Disclosure elements are supported in most modern browsers except IE, Edge (under consideration), and Opera Mini (cf. https://caniuse.com/#search=details). As it is, IE, Edge, and Opera Mini fallback gracefully by disclosing the nested content.

Steps to reproduce:

  1. Type the following in the HTML editor (e.g., on a course Page)
<details>
  <summary>Test Summary</summary>
  Test details
</details>
  1. Save and Publish the Page

Expected behavior:

Entering <details> and <summary> elements in the HTML editor should yield working disclosure widget in the Rich Text editor and published Pages. Also, <details> elements without the open attribute should appear closed by default.

Actual behavior:

After entering the valid HTML and switching to the Rich Content Editor to preview; the details disclosure widget is toggled open (rather than closed by default, as expected) and the toggle function does not work.

image

Furthermore, view the published page and the <details> and <summary> appears to be escaped entirely:

image

<!-- From browser inspector -->
<div class="show-content user_content clearfix enhanced">
  <h1 class="page-title">Test Details</h1>

Test Summary
Test details

</div>

Additional notes:

HTML Living Standard

Mozilla Developer Network

cf. Scott O’Hara on accessiblity support for <details> and <summary> elements.

simonista commented 4 years ago

Hi @reubenlillie, thanks for the detailed bug report and sorry to be slow getting back to you.

re: the rendering inside the rich text editor, it looks like this is actually a problem with Tinymce (which drives our editor), as you can see by trying the same test on their side: https://www.tiny.cloud/. So I would recommend sending the rendering issue their way.

re: the tags not being there after saving, that is because we have a list of all allowed tags from the editor, and anything not in the list is filtered out. You can see that list here: https://github.com/instructure/canvas-lms/blob/master/gems/canvas_sanitize/lib/canvas_sanitize/canvas_sanitize.rb. I'll mention to the editor team that you'd like to see these tags get allowed, and see if they've had other requests for them!