Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Additional lesson preview accessibility #2828

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

brianhogg
Copy link
Contributor

@brianhogg brianhogg commented Dec 9, 2024

Description

Adds tooltip text as a screen-reader only element and indicates the link is disabled when a lesson cannot be accessed.

Fixes #2827

How has this been tested?

Manually

Screenshots

CleanShot.2024-12-09.at.10.08.31.mp4

Checklist:

  • This PR requires and contains at least one changelog file.
  • My code has been tested.
  • My code passes all existing automated tests.
  • My code follows the LifterLMS Coding & Documentation Standards.

@@ -109,7 +109,7 @@
*/
get_tooltip: function( msg ) {
var $el = $( '<div class="llms-tooltip" />' );
$el.append( '<div class="llms-tooltip-content">' + msg + '</div>' );
$el.append( '<div class="llms-tooltip-content" aria-hidden="true">' + msg + '</div>' );

Check warning

Code scanning / CodeQL

DOM text reinterpreted as HTML Medium

DOM text
is reinterpreted as HTML without escaping meta-characters.

Copilot Autofix AI 4 days ago

To fix the problem, we need to ensure that the msg variable is properly escaped before being inserted into the HTML. This can be achieved by using a function that escapes HTML special characters, thereby preventing any potential XSS attacks.

The best way to fix this without changing existing functionality is to create a utility function that escapes HTML special characters and use this function to sanitize the msg variable before it is used in the HTML construction.

Suggested changeset 1
assets/js/app/llms-lesson-preview.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/assets/js/app/llms-lesson-preview.js b/assets/js/app/llms-lesson-preview.js
--- a/assets/js/app/llms-lesson-preview.js
+++ b/assets/js/app/llms-lesson-preview.js
@@ -111,5 +111,20 @@
 		var $el = $( '<div class="llms-tooltip" />' );
-		$el.append( '<div class="llms-tooltip-content" aria-hidden="true">' + msg + '</div>' );
+		$el.append( '<div class="llms-tooltip-content" aria-hidden="true">' + this.escapeHtml(msg) + '</div>' );
 		return $el;
 	},
+
+	/**
+	 * Escape HTML special characters in a string
+	 *
+	 * @param    string   str   string to escape
+	 * @return   string
+	 * @since    3.16.12
+	 */
+	escapeHtml: function( str ) {
+		return str.replace(/&/g, '&amp;')
+		          .replace(/</g, '&lt;')
+		          .replace(/>/g, '&gt;')
+		          .replace(/"/g, '&quot;')
+		          .replace(/'/g, '&#039;');
+	},
 
EOF
@@ -111,5 +111,20 @@
var $el = $( '<div class="llms-tooltip" />' );
$el.append( '<div class="llms-tooltip-content" aria-hidden="true">' + msg + '</div>' );
$el.append( '<div class="llms-tooltip-content" aria-hidden="true">' + this.escapeHtml(msg) + '</div>' );
return $el;
},

/**
* Escape HTML special characters in a string
*
* @param string str string to escape
* @return string
* @since 3.16.12
*/
escapeHtml: function( str ) {
return str.replace(/&/g, '&amp;')
.replace(/</g, '&lt;')
.replace(/>/g, '&gt;')
.replace(/"/g, '&quot;')
.replace(/'/g, '&#039;');
},

Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Awaiting Review
Development

Successfully merging this pull request may close these issues.

1 participant