instructure / canvas-lms

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

Unable to set width/height on <source> in <picture> #2272

Closed buckett closed 9 months ago

buckett commented 11 months ago

Summary:

Canvas allows the <picture> in wiki pages and this can be used to provide different images depending on the end users device. This is useful as decorative images can be reduced or hidden when on a small screen device. This all works, however Canvas doesn't allow the width/height attributes on <source> tags and this means that the browser doesn't know how much space to reserve for the image until it has loaded the image. This results in a browser relayout once the image has loaded. If the user is on a mobile device (or any device with a poor connection) this can happen quite a while after the page has loaded and means content can end up shuffling around on the page.

Steps to reproduce:

  1. Go into source view in the RCE and paste in:
    <picture>
      <source srcset="https://interactive-examples.mdn.mozilla.net/media/cc0-images/surfer-240-200.jpg" width="240" height="200" media="(max-width: 600px)"/>
      <img src="https://interactive-examples.mdn.mozilla.net/media/cc0-images/painted-hand-298-332.jpg" width="298" height="332" alt="" />
    </picture>
    <p>Text after the image</p>
  2. Save the page and see that the width/height attributes have been stripped from the <source> tag.
  3. Load the page over a very slow connection and notice the "Text after the image" moves down the page once the image loads when the browser is less than 600 pixels wide.

Expected behavior:

The width/height should be allowed through the RCE and Canvas filtering and the browser then reserves space in the rendering.

Actual behavior:

The width/height are stripped.

Additional notes:

These attributes are documented on https://developer.mozilla.org/en-US/docs/Web/HTML/Element/source These attribute are already allowed for <img> tags. The original support for <picture> was done in https://github.com/instructure/canvas-lms/commit/205e5420fce7b6da0b73f365161f2f5ebfbe9035 which has a reference of MAT-89

jakeoeding commented 9 months ago

This has been added as part of https://github.com/instructure/canvas-lms/commit/90134300d2113ef3cf442acc9e32b46dd985e3a6

buckett commented 9 months ago

Magic, thanks @jakeoeding. 🎉