fix: add target='_blank' for videos hosted outside edx (#36361)

Author: hinakhadim

lms/djangoapps/courseware/tests/test_video_mongo.py

@@ -96,6 +96,7 @@ def test_video_constructor(self):
             'cdn_exp_group': None,
             'display_name': 'A Name',
             'download_video_link': 'example.mp4',
+            'is_video_from_same_origin': False,
             'handout': None,
             'hide_downloads': False,
             'id': self.block.location.html_id(),
@@ -184,6 +185,7 @@ def test_video_constructor(self):
             'cdn_exp_group': None,
             'display_name': 'A Name',
             'download_video_link': 'example.mp4',
+            'is_video_from_same_origin': False,
             'handout': None,
             'hide_downloads': False,
             'is_embed': False,
@@ -460,6 +462,7 @@ def test_get_html_track(self):
             'cdn_exp_group': None,
             'display_name': 'A Name',
             'download_video_link': 'example.mp4',
+            'is_video_from_same_origin': False,
             'handout': None,
             'hide_downloads': False,
             'id': self.block.location.html_id(),
@@ -592,6 +595,7 @@ def test_get_html_source(self):
             'cdn_exp_group': None,
             'display_name': 'A Name',
             'download_video_link': 'example.mp4',
+            'is_video_from_same_origin': False,
             'handout': None,
             'hide_downloads': False,
             'id': self.block.location.html_id(),
@@ -730,6 +734,7 @@ def test_get_html_with_mocked_edx_video_id(self):
             'cdn_exp_group': None,
             'display_name': 'A Name',
             'download_video_link': 'example.mp4',
+            'is_video_from_same_origin': False,
             'handout': None,
             'hide_downloads': False,
             'is_embed': False,
@@ -809,12 +814,16 @@ def test_get_html_with_existing_edx_video_id(self):
             'edx_video_id': edx_video_id,
             'result': {
                 'download_video_link': f'http://fake-video.edx.org/{edx_video_id}.mp4',
-                'sources': ['example.mp4', 'example.webm'] + [video['url'] for video in encoded_videos],
+                'is_video_from_same_origin': True,
+                'sources': ['http://fake-video.edx.org/example.mp4', 'http://fake-video.edx.org/example.webm'] +
+                           [video['url'] for video in encoded_videos],
             },
         }
-        # context returned by get_html when provided with above data
-        # expected_context, a dict to assert with context
-        context, expected_context = self.helper_get_html_with_edx_video_id(data)
+        with override_settings(VIDEO_CDN_URL={'default': 'http://fake-video.edx.org'}):
+            # context returned by get_html when provided with above data
+            # expected_context, a dict to assert with context
+            context, expected_context = self.helper_get_html_with_edx_video_id(data)
+
         mako_service = self.block.runtime.service(self.block, 'mako')
         assert get_context_dict_from_string(context) ==\
                get_context_dict_from_string(mako_service.render_lms_template('video.html', expected_context))
@@ -839,12 +848,15 @@ def test_get_html_with_existing_unstripped_edx_video_id(self):
             'edx_video_id': f"{edx_video_id}\t",
             'result': {
                 'download_video_link': f'http://fake-video.edx.org/{edx_video_id}.mp4',
-                'sources': ['example.mp4', 'example.webm'] + [video['url'] for video in encoded_videos],
+                'is_video_from_same_origin': True,
+                'sources': ['http://fake-video.edx.org/example.mp4', 'http://fake-video.edx.org/example.webm'] +
+                           [video['url'] for video in encoded_videos],
             },
         }
-        # context returned by get_html when provided with above data
-        # expected_context, a dict to assert with context
-        context, expected_context = self.helper_get_html_with_edx_video_id(data)
+        with override_settings(VIDEO_CDN_URL={'default': 'http://fake-video.edx.org'}):
+            # context returned by get_html when provided with above data
+            # expected_context, a dict to assert with context
+            context, expected_context = self.helper_get_html_with_edx_video_id(data)
 
         mako_service = self.block.runtime.service(self.block, 'mako')
         assert get_context_dict_from_string(context) ==\
@@ -910,6 +922,7 @@ def helper_get_html_with_edx_video_id(self, data):
             'cdn_exp_group': None,
             'display_name': 'A Name',
             'download_video_link': 'example.mp4',
+            'is_video_from_same_origin': False,
             'handout': None,
             'hide_downloads': False,
             'is_embed': False,
@@ -951,6 +964,7 @@ def helper_get_html_with_edx_video_id(self, data):
             'block_id': str(self.block.location),
             'course_id': str(self.block.location.course_key),
             'download_video_link': data['result']['download_video_link'],
+            'is_video_from_same_origin': data['result']['is_video_from_same_origin'],
             'metadata': json.dumps(expected_context['metadata'])
         })
         return context, expected_context
@@ -1029,6 +1043,7 @@ def side_effect(*args, **kwargs):  # lint-amnesty, pylint: disable=unused-argume
             'cdn_exp_group': None,
             'display_name': 'A Name',
             'download_video_link': None,
+            'is_video_from_same_origin': False,
             'handout': None,
             'hide_downloads': False,
             'is_embed': False,
@@ -1129,6 +1144,7 @@ def test_get_html_cdn_source_external_video(self):
             'cdn_exp_group': None,
             'display_name': 'A Name',
             'download_video_link': None,
+            'is_video_from_same_origin': False,
             'handout': None,
             'hide_downloads': False,
             'id': None,
@@ -2382,6 +2398,7 @@ def test_bumper_metadata(self, get_url_for_profiles, get_bumper_settings, is_bum
             'cdn_exp_group': None,
             'display_name': 'A Name',
             'download_video_link': 'example.mp4',
+            'is_video_from_same_origin': False,
             'handout': None,
             'hide_downloads': False,
             'is_embed': False,
@@ -2464,6 +2481,7 @@ def prepare_expected_context(self, autoadvanceenabled_flag, autoadvance_flag):
             'cdn_exp_group': None,
             'display_name': 'A Name',
             'download_video_link': 'example.mp4',
+            'is_video_from_same_origin': False,
             'handout': None,
             'hide_downloads': False,
             'is_embed': False,

lms/templates/video.html

@@ -57,7 +57,7 @@ <h3 class="hd hd-4 downloads-heading sr" id="video-download-transcripts_${id}">$
                     <h4 class="hd hd-5">${_('Video')}</h4>
                     % if download_video_link:
                         <span class="icon fa fa-download" aria-hidden="true"></span>
-                        <a class="btn-link video-sources video-download-button" href="${download_video_link}">
+                        <a class="btn-link video-sources video-download-button" href="${download_video_link}" target="${'_blank' if not is_video_from_same_origin else '_self'}">
                             ${_('Download video file')}
                         </a>
                     % endif

xmodule/video_block/video_block.py

@@ -471,6 +471,8 @@ def get_html(self, view=STUDENT_VIEW, context=None):  # lint-amnesty, pylint: di
 
         bumperize(self)
 
+        is_video_from_same_origin = bool(download_video_link and cdn_url and download_video_link.startswith(cdn_url))
+
         template_context = {
             'autoadvance_enabled': autoadvance_enabled,
             'branding_info': branding_info,
@@ -479,6 +481,7 @@ def get_html(self, view=STUDENT_VIEW, context=None):  # lint-amnesty, pylint: di
             'cdn_exp_group': cdn_exp_group,
             'display_name': self.display_name_with_default,
             'download_video_link': download_video_link,
+            'is_video_from_same_origin': is_video_from_same_origin,
             'handout': self.handout,
             'hide_downloads': is_public_view or is_embed,
             'id': self.location.html_id(),