From 4e39b31711fa1c445733ebabfe4acc3a7bb9e614 Mon Sep 17 00:00:00 2001 From: Ed Schiebel Date: Thu, 27 Feb 2020 16:27:26 -0500 Subject: [PATCH] Add a title to embedded media's dom element which is typically an iframe closes LA-628 flag=rce_enhancements test plan: - in the rce, choose Media > Upload/Record Media, the the Embed tab, then paste in some html (you can get a real video from youtube, or just type any old valid html element code) - click submit - click save > expect the element you added to have a title="embedded content" - repeat, but when you enter the embed code, give the embedded html an aria-label or title attribute - submit and save > expect the embedded element to have kept its original title or aria-label Change-Id: I42a8ee26e8a95aa255705f09efcf581de69664d5 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/228273 Tested-by: Service Cloud Jenkins Reviewed-by: Jeremy Stanley QA-Review: Jeremy Stanley Product-Review: Ed Schiebel --- packages/canvas-rce/src/rce/RCEWrapper.js | 25 +++++++++++++++++++---- spec/selenium/rcs/pages/rce_next_page.rb | 17 +++++++++++++++ spec/selenium/rcs/rce_next_spec.rb | 23 ++++++++++++++++++++- 3 files changed, 60 insertions(+), 5 deletions(-) diff --git a/packages/canvas-rce/src/rce/RCEWrapper.js b/packages/canvas-rce/src/rce/RCEWrapper.js index f884bc342ee..b4f2064f6a1 100644 --- a/packages/canvas-rce/src/rce/RCEWrapper.js +++ b/packages/canvas-rce/src/rce/RCEWrapper.js @@ -329,12 +329,29 @@ class RCEWrapper extends React.Component { this.contentInserted(element) } - // inserting an iframe in tinymce (as is often the case with - // embedded content) causes it to wrap it in a span - // and it's often inserted into a

on top of that. Find the - // iframe and use it to flash the indicator. insertEmbedCode(code) { const editor = this.mceInstance() + // tinymce treats iframes uniquely, and doesn't like adding attributes + // once it's in the editor, and I'd rather not parse the incomming html + // string with a regex, so let's create a temp copy, then add a title + // attribute if one doesn't exist. This will let screenreaders announce + // that there's some embedded content helper + // From what I've read, "title" is more reliable than "aria-label" for + // elements like iframes and embeds. + const temp = document.createElement('div') + temp.innerHTML = code + const code_elem = temp.firstElementChild + if (code_elem) { + if (!code_elem.hasAttribute('title') && !code_elem.hasAttribute('aria-label')) { + code_elem.setAttribute('title', formatMessage('embedded content')) + } + code = code_elem.outerHTML + } + + // inserting an iframe in tinymce (as is often the case with + // embedded content) causes it to wrap it in a span + // and it's often inserted into a

on top of that. Find the + // iframe and use it to flash the indicator. const element = contentInsertion.insertContent(editor, code) const ifr = element && element.querySelector && element.querySelector('iframe') if (ifr) { diff --git a/spec/selenium/rcs/pages/rce_next_page.rb b/spec/selenium/rcs/pages/rce_next_page.rb index da89b763fe7..4a22c44abb1 100644 --- a/spec/selenium/rcs/pages/rce_next_page.rb +++ b/spec/selenium/rcs/pages/rce_next_page.rb @@ -473,6 +473,11 @@ module RCENextPage wait_for_ajaximations end + def click_embed_media_tab + fj('[role="tab"]:contains("Embed")').click + wait_for_animations + end + def click_course_media course_media.click wait_for_ajaximations @@ -618,6 +623,10 @@ module RCENextPage decorative_options_checkbox.click end + def click_upload_media_submit_button + upload_media_submit_button.click + end + def user_media_menu_item fj('[role="menuitem"]:contains("User Media")') end @@ -629,4 +638,12 @@ module RCENextPage def menu_item_by_menu_id(menu_id, item_label) fj("##{menu_id}:contains('#{item_label}')") end + + def embed_code_textarea + f('textarea[placeholder="Embed Code"]') + end + + def upload_media_submit_button + f('[aria-label="Upload Media"] button[type="submit"]') + end end diff --git a/spec/selenium/rcs/rce_next_spec.rb b/spec/selenium/rcs/rce_next_spec.rb index b6280bb5a20..195ec48f95d 100644 --- a/spec/selenium/rcs/rce_next_spec.rb +++ b/spec/selenium/rcs/rce_next_spec.rb @@ -499,7 +499,7 @@ describe "RCE next tests" do expect(upload_document_modal).to be_displayed end - it "schould include edia upload option if kaltura is enabled" do + it "should include media upload option if kaltura is enabled" do double('CanvasKaltura::ClientV3') allow(CanvasKaltura::ClientV3).to receive(:config).and_return({}) visit_front_page_edit(@course) @@ -559,6 +559,27 @@ describe "RCE next tests" do expect(f('body')).not_to contain_css('[data-testid="CanvasContentTray"]') end + it "should add a title attribute to an inserted iframe" do + # as typically happens when embedding media, like a youtube video + double('CanvasKaltura::ClientV3') + allow(CanvasKaltura::ClientV3).to receive(:config).and_return({}) + visit_front_page_edit(@course) + + click_media_toolbar_button + click_upload_media + click_embed_media_tab + code_box = embed_code_textarea + code_box.click + code_box.send_keys('') + click_upload_media_submit_button + wait_for_animations + + fj('button:contains("Save")').click # save the page + wait_for_ajaximations + + expect(f('iframe[title="embedded content"][src="https://example.com/"]')).to be_displayed + end + describe "keyboard shortcuts" do it "should open keyboard shortcut modal with alt-f8" do visit_front_page_edit(@course)