diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index ec3427c690..b3d1196bb5 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -58,7 +58,8 @@ from lms.djangoapps.courseware.tests.helpers import get_expiration_banner_text from lms.djangoapps.courseware.testutils import RenderXBlockTestMixin from lms.djangoapps.courseware.toggles import ( COURSEWARE_MICROFRONTEND_COURSE_TEAM_PREVIEW, - REDIRECT_TO_COURSEWARE_MICROFRONTEND + COURSEWARE_OPTIMIZED_RENDER_XBLOCK, + REDIRECT_TO_COURSEWARE_MICROFRONTEND, ) from lms.djangoapps.courseware.url_helpers import get_microfrontend_url, get_redirect_url from lms.djangoapps.courseware.user_state_client import DjangoXBlockUserStateClient @@ -3448,3 +3449,104 @@ class MFERedirectTests(BaseViewsTestCase): # lint-amnesty, pylint: disable=miss with override_waffle_flag(REDIRECT_TO_COURSEWARE_MICROFRONTEND, active=True): assert self.client.get(lms_url).status_code == 200 + + +class ContentOptimizationTestCase(ModuleStoreTestCase): + """ + Test our ability to make browser optimizations based on XBlock content. + """ + def setUp(self): + super().setUp() + self.math_html_usage_keys = [] + + with self.store.default_store(ModuleStoreEnum.Type.split): + self.course = CourseFactory.create(display_name=u'teꜱᴛ course', run="Testing_course") + with self.store.bulk_operations(self.course.id): + chapter = ItemFactory.create( + category='chapter', + parent_location=self.course.location, + display_name="Chapter 1", + ) + section = ItemFactory.create( + category='sequential', + parent_location=chapter.location, + due=datetime(2013, 9, 18, 11, 30, 00), + display_name='Sequential 1', + format='Homework' + ) + self.math_vertical = ItemFactory.create( + category='vertical', + parent_location=section.location, + display_name='Vertical with Mathjax HTML', + ) + self.no_math_vertical = ItemFactory.create( + category='vertical', + parent_location=section.location, + display_name='Vertical with No Mathjax HTML', + ) + MATHJAX_TAG_PAIRS = [ + (r"\(", r"\)"), + (r"\[", r"\]"), + ("[mathjaxinline]", "[/mathjaxinline]"), + ("[mathjax]", "[/mathjax]"), + ] + for (i, (start_tag, end_tag)) in enumerate(MATHJAX_TAG_PAIRS): + math_html_block = ItemFactory.create( + category='html', + parent_location=self.math_vertical.location, + display_name=f"HTML With Mathjax {i}", + data=f"
Hello Math! {start_tag}x^2 + y^2{end_tag}
", + ) + self.math_html_usage_keys.append(math_html_block.location) + + self.html_without_mathjax = ItemFactory.create( + category='html', + parent_location=self.no_math_vertical.location, + display_name="HTML Without Mathjax", + data="I talk about mathjax, but I have no actual Math!
", + ) + + self.course_key = self.course.id + self.user = UserFactory(username='staff_user', profile__country='AX', is_staff=True) + self.date = datetime(2013, 1, 22, tzinfo=UTC) + self.enrollment = CourseEnrollment.enroll(self.user, self.course_key) + self.enrollment.created = self.date + self.enrollment.save() + + @override_waffle_flag(COURSEWARE_OPTIMIZED_RENDER_XBLOCK, True) + def test_mathjax_detection(self): + self.client.login(username=self.user.username, password=TEST_PASSWORD) + + # Check the HTML blocks with Math + for usage_key in self.math_html_usage_keys: + url = reverse("render_xblock", kwargs={'usage_key_string': str(usage_key)}) + response = self.client.get(url) + assert response.status_code == 200 + assert b"MathJax.Hub.Config" in response.content + + # Check the one without Math... + url = reverse("render_xblock", kwargs={ + 'usage_key_string': str(self.html_without_mathjax.location) + }) + response = self.client.get(url) + assert response.status_code == 200 + assert b"MathJax.Hub.Config" not in response.content + + # The containing vertical should still return MathJax (for now) + url = reverse("render_xblock", kwargs={ + 'usage_key_string': str(self.no_math_vertical.location) + }) + response = self.client.get(url) + assert response.status_code == 200 + assert b"MathJax.Hub.Config" in response.content + + @override_waffle_flag(COURSEWARE_OPTIMIZED_RENDER_XBLOCK, False) + def test_mathjax_detection_disabled(self): + """Check that we can disable optimizations.""" + self.client.login(username=self.user.username, password=TEST_PASSWORD) + url = reverse("render_xblock", kwargs={ + 'usage_key_string': str(self.html_without_mathjax.location) + }) + response = self.client.get(url) + assert response.status_code == 200 + assert b"MathJax.Hub.Config" in response.content diff --git a/lms/djangoapps/courseware/toggles.py b/lms/djangoapps/courseware/toggles.py index 46ac561abd..46be367250 100644 --- a/lms/djangoapps/courseware/toggles.py +++ b/lms/djangoapps/courseware/toggles.py @@ -115,6 +115,21 @@ EXAM_RESUME_PROCTORING_IMPROVEMENTS = CourseWaffleFlag( WAFFLE_FLAG_NAMESPACE, 'exam_resume_proctoring_improvements', __name__ ) +# .. toggle_name: courseware.optimized_render_xblock +# .. toggle_implementation: CourseWaffleFlag +# .. toggle_default: False +# .. toggle_description: Waffle flag that determines whether we speed up the render_xblock for browsers by +# removing unnecessary JavaScript and CSS. It is possible that this could introduce edge cases with content +# that relies on these assets, so being a CourseWaffleFlag will give us the flexibility to exempt courses +# from these optimizations. +# .. toggle_use_cases: temporary +# .. toggle_creation_date: 2021-02-09 +# .. toggle_target_removal_date: 2021-05-01 +# .. toggle_warnings: None +COURSEWARE_OPTIMIZED_RENDER_XBLOCK = CourseWaffleFlag( + WAFFLE_FLAG_NAMESPACE, 'optimized_render_xblock', __name__ +) + def course_exit_page_is_active(course_key): return ( diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index b292f3ce9d..acecda81cd 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -33,7 +33,7 @@ from django.views.decorators.csrf import ensure_csrf_cookie from django.views.decorators.http import require_GET, require_http_methods, require_POST from django.views.generic import View from edx_django_utils import monitoring as monitoring_utils -from edx_django_utils.monitoring import set_custom_attributes_for_course_key +from edx_django_utils.monitoring import set_custom_attribute, set_custom_attributes_for_course_key from ipware.ip import get_ip from markupsafe import escape from opaque_keys import InvalidKeyError @@ -135,6 +135,7 @@ from ..context_processor import user_timezone_locale_prefs from ..entrance_exams import user_can_skip_entrance_exam from ..module_render import get_module, get_module_by_usage_id, get_module_for_descriptor from ..tabs import _get_dynamic_tabs +from ..toggles import COURSEWARE_OPTIMIZED_RENDER_XBLOCK log = logging.getLogger("edx.courseware") @@ -1669,6 +1670,11 @@ def render_xblock(request, usage_key_string, check_if_enrolled=True): usage_key = usage_key.replace(course_key=modulestore().fill_in_run(usage_key.course_key)) course_key = usage_key.course_key + # Gathering metrics to make performance measurements easier. + set_custom_attributes_for_course_key(course_key) + set_custom_attribute('usage_key', usage_key_string) + set_custom_attribute('block_type', usage_key.block_type) + requested_view = request.GET.get('view', 'student_view') if requested_view != 'student_view' and requested_view != 'public_view': # lint-amnesty, pylint: disable=consider-using-in return HttpResponseBadRequest( @@ -1747,8 +1753,11 @@ def render_xblock(request, usage_key_string, check_if_enrolled=True): ) ) + fragment = block.render(requested_view, context=student_view_context) + optimization_flags = get_optimization_flags_for_content(block, fragment) + context = { - 'fragment': block.render(requested_view, context=student_view_context), + 'fragment': fragment, 'course': course, 'disable_accordion': True, 'allow_iframing': True, @@ -1768,10 +1777,82 @@ def render_xblock(request, usage_key_string, check_if_enrolled=True): 'is_learning_mfe': is_learning_mfe, 'is_mobile_app': is_request_from_mobile_app(request), 'reset_deadlines_url': reverse(RESET_COURSE_DEADLINES_NAME), + + **optimization_flags, } return render_to_response('courseware/courseware-chromeless.html', context) +def get_optimization_flags_for_content(block, fragment): + """ + Return a dict with a set of display options appropriate for the block. + + This is going to start in a very limited way. + """ + safe_defaults = { + 'enable_mathjax': True + } + + # Only run our optimizations on the leaf HTML and ProblemBlock nodes. The + # mobile apps access these directly, and we don't have to worry about + # XBlocks that dynamically load content, like inline discussions. + usage_key = block.location + + # For now, confine ourselves to optimizing just the HTMLBlock + if usage_key.block_type != 'html': + return safe_defaults + + if not COURSEWARE_OPTIMIZED_RENDER_XBLOCK.is_enabled(usage_key.course_key): + return safe_defaults + + inspector = XBlockContentInspector(block, fragment) + flags = dict(safe_defaults) + flags['enable_mathjax'] = inspector.has_mathjax_content() + + return flags + + +class XBlockContentInspector: + """ + Class to inspect rendered XBlock content to determine dependencies. + + A lot of content has been written with the assumption that certain + JavaScript and assets are available. This has caused us to continue to + include these assets in the render_xblock view, despite the fact that they + are not used by the vast majority of content. + + In order to try to provide faster load times for most users on most content, + this class has the job of detecting certain patterns in XBlock content that + would imply these dependencies, so we know when to include them or not. + """ + def __init__(self, block, fragment): + self.block = block + self.fragment = fragment + + def has_mathjax_content(self): + """ + Returns whether we detect any MathJax in the fragment. + + Note that this only works for things that are rendered up front. If an + XBlock is capable of modifying the DOM afterwards to inject math content + into the page, this will not catch it. + """ + # The following pairs are used to mark Mathjax syntax in XBlocks. There + # are other options for the wiki, but we don't worry about those here. + MATHJAX_TAG_PAIRS = [ + (r"\(", r"\)"), + (r"\[", r"\]"), + ("[mathjaxinline]", "[/mathjaxinline]"), + ("[mathjax]", "[/mathjax]"), + ] + content = self.fragment.body_html() + for (start_tag, end_tag) in MATHJAX_TAG_PAIRS: + if start_tag in content and end_tag in content: + return True + + return False + + # Translators: "percent_sign" is the symbol "%". "platform_name" is a # string identifying the name of this installation, such as "edX". FINANCIAL_ASSISTANCE_HEADER = _( diff --git a/lms/templates/courseware/courseware-chromeless.html b/lms/templates/courseware/courseware-chromeless.html index 4e61397f80..929b002cb5 100644 --- a/lms/templates/courseware/courseware-chromeless.html +++ b/lms/templates/courseware/courseware-chromeless.html @@ -59,7 +59,9 @@ ${static.get_page_title_breadcrumbs(course_name())} <%static:js group='courseware'/> - <%include file="/mathjax_include.html" args="disable_fast_preview=True"/> + % if enable_mathjax: + <%include file="/mathjax_include.html" args="disable_fast_preview=True"/> + % endif % if staff_access: <%include file="xqa_interface.html"/> % endif