-
Notifications
You must be signed in to change notification settings - Fork 5
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
fix: update html cleaning rules #444
Conversation
@ziafazal Please review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ahmed-arb overall looks good few minor things to look into. Please add unit tests too.
a55f99b
to
592c7e3
Compare
@ziafazal please review again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ahmed-arb completed another round of review.
@@ -20,7 +20,6 @@ class MessengerConfig(AppConfig): | |||
PluginSettings.CONFIG: { | |||
ProjectType.LMS: { | |||
SettingsType.COMMON: {PluginSettings.RELATIVE_PATH: 'settings.common'}, | |||
SettingsType.TEST: {PluginSettings.RELATIVE_PATH: 'settings.test'}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why we need to remove this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This module does not exist.
ec0572d
to
c37c050
Compare
@ziafazal Please review now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ahmed-arb this looks good now. 👍
Context:
OpenEdx uses Cleaner from LXML to sanitize HTML before displaying it on the about page. It removes URLs if they contain JavaScript. The decision rule matches the following schemes in the URL:
Solution:
We override the rule to bypass cleaning if
href
value is a URL.