instructure / canvas-lms

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

Error in calculating canvas_chrome_height since jQuery update #2337

Closed jonespm closed 2 months ago

jonespm commented 2 months ago

There's a bug with setting the inline tool height after the update to jQuery 3. I believe this is because of the [change from jQuery](https://stackoverflow.com/questions/41454285/jquery-outerheight-returns-undefined-instead-of-null#:~:text=outerHeight()%20returns%20undefined%20instead%20of%20null%20if%20called%20on,%24('%23div1').) returning undefined now instead of null before making the calculation NaN.

Now canvas_chrome_height in this code is NaN and previously it was 72.

This is partially because the element "footer" no longer is on the document but could also have a || 0 guard on it.

This code also calls $('#sequence_footer').outerHeight(true) which should probably also be protected as suggested like $('#sequence_footer').outerHeight(true) || 0

I believe this may be either causing or related to the iframe resizing issues for LTI tools.

https://github.com/instructure/canvas-lms/blame/ce2e02fb0dd495c442fe8e775e08bc49292203d8/ui/features/external_tools_show/jquery/tool_inline.js#L131

Thanks!

tucker-m commented 2 months ago

Hi @jonespm! Thank you for bringing this to our attention and digging into the cause. We have just merged and deployed a fix for it.