diff --git a/docs/topics/api/addons.rst b/docs/topics/api/addons.rst index 7fa0527f62e7..727ffe6ab7e2 100644 --- a/docs/topics/api/addons.rst +++ b/docs/topics/api/addons.rst @@ -162,7 +162,7 @@ This endpoint allows you to fetch a specific add-on by id, slug or guid. :>json string created: The date the add-on was created. :>json object current_version: Object holding the current :ref:`version ` of the add-on. For performance reasons the ``license`` field omits the ``text`` property from both the search and detail endpoints. :>json string default_locale: The add-on default locale for translations. - :>json object|null description: The add-on description (See :ref:`translated fields `). This field might contain some HTML tags. + :>json object|null description: The add-on description (See :ref:`translated fields `). This field might contain markdown. :>json object|null developer_comments: Additional information about the add-on provided by the developer. (See :ref:`translated fields `). :>json string edit_url: The URL to the developer edit page for the add-on. :>json string guid: The add-on `extension identifier `_. @@ -203,7 +203,7 @@ This endpoint allows you to fetch a specific add-on by id, slug or guid. :>json string review_url: The URL to the reviewer review page for the add-on. :>json string slug: The add-on slug. :>json string status: The :ref:`add-on status `. - :>json object|null summary: The add-on summary (See :ref:`translated fields `). This field supports "linkification" and therefore might contain HTML hyperlinks. + :>json object|null summary: The add-on summary (See :ref:`translated fields `). This field supports "linkification" and therefore might contain Markdown hyperlinks. :>json object|null support_email: The add-on support email (See :ref:`translated fields `). :>json object|null support_url: The add-on support URL (See :ref:`translated fields ` and :ref:`Outgoing Links `). :>json array tags: List containing the tag names set on the add-on. @@ -311,7 +311,7 @@ is compatible with. :` the add-on belongs to for a given :ref:`add-on application `. :`_. :` and custom license text is fixed to `en-US`. - :`). This field can contain some HTML tags. + :`). This field can contain some Markdown. :`). :` and :ref:`Outgoing Links `). :` the add-on belongs to for a given :ref:`add-on application `. :`_. :` and custom license text is fixed to `en-US`. - :`). This field can contain some HTML tags. + :`). This field can contain some Markdown. :`). :` and :ref:`Outgoing Links `). :` to upload a new icon. diff --git a/docs/topics/api/overview.rst b/docs/topics/api/overview.rst index 18b5e30d4723..311dd68b60d9 100644 --- a/docs/topics/api/overview.rst +++ b/docs/topics/api/overview.rst @@ -246,7 +246,7 @@ Note, if the field is also a translated field then the ``url`` and ``outgoing`` values could be an object rather than a string (See :ref:`translated fields ` for translated field representations). -Fields supporting some HTML, such as add-on ``description`` or ``summary``, +Fields supporting some HTML or Markdown, such as add-on ``description`` or ``summary``, always wrap any links directly inside the content (the original url is not available). diff --git a/requirements/prod.txt b/requirements/prod.txt index 844b006bd1f8..234c3a954e63 100644 --- a/requirements/prod.txt +++ b/requirements/prod.txt @@ -1208,3 +1208,6 @@ watchdog[watchmedo]==3.0.0 \ django-node-assets==0.9.14 \ --hash=sha256:80cbe3d10521808309712b2aa5ef6d69799bbcafef844cf7f223d3c93f405768 \ --hash=sha256:d5b5c472136084d533268f52ab77897327863a102e25c81f484aae85eb806987 +Markdown==3.7 \ + --hash=sha256:2ae2471477cfd02dbbf038d5d9bc226d40def84b4fe2986e49b59b6b472bbed2 \ + --hash=sha256:7eb6df5690b81a1d7942992c97fad2938e956e79df20cbc6186e9c3a77b1c803 diff --git a/src/olympia/addons/models.py b/src/olympia/addons/models.py index 8125e9ce7678..b9dc5fdb8672 100644 --- a/src/olympia/addons/models.py +++ b/src/olympia/addons/models.py @@ -65,7 +65,7 @@ from olympia.tags.models import Tag from olympia.translations.fields import ( NoURLsField, - PurifiedField, + PurifiedMarkdownField, TranslatedField, save_signal, ) @@ -501,12 +501,14 @@ class Addon(OnChangeMixin, ModelBase): homepage = TranslatedField(max_length=255) support_email = TranslatedField(db_column='supportemail', max_length=100) support_url = TranslatedField(db_column='supporturl', max_length=255) - description = PurifiedField(short=False, max_length=15000) + description = PurifiedMarkdownField(short=False, max_length=15000) summary = NoURLsField(max_length=250) - developer_comments = PurifiedField(db_column='developercomments', max_length=3000) - eula = PurifiedField(max_length=350000) - privacy_policy = PurifiedField(db_column='privacypolicy', max_length=150000) + developer_comments = PurifiedMarkdownField( + db_column='developercomments', max_length=3000 + ) + eula = PurifiedMarkdownField(max_length=350000) + privacy_policy = PurifiedMarkdownField(db_column='privacypolicy', max_length=150000) average_rating = models.FloatField( max_length=255, default=0, null=True, db_column='averagerating' diff --git a/src/olympia/addons/tests/test_models.py b/src/olympia/addons/tests/test_models.py index a6c7a17e6806..2375a02953e8 100644 --- a/src/olympia/addons/tests/test_models.py +++ b/src/olympia/addons/tests/test_models.py @@ -964,208 +964,150 @@ def newlines_helper(self, string_before): addon.save() return addon.privacy_policy.localized_string_clean - def test_newlines_normal(self): + def test_newlines(self): before = ( 'Paragraph one.\n' 'This should be on the very next line.\n\n' "Should be two nl's before this line.\n\n\n" - "Should be three nl's before this line.\n\n\n\n" - "Should be four nl's before this line." + "Should be two nl's before this line.\n\n\n\n" + "Should be two nl's before this line." ) - after = before # Nothing special; this shouldn't change. + # Markdown treats multiple blank lines as one. + after = ( + 'Paragraph one.\n' + 'This should be on the very next line.\n\n' + "Should be two nl's before this line.\n\n" + "Should be two nl's before this line.\n\n" + "Should be two nl's before this line." + ) assert self.newlines_helper(before) == after - def test_newlines_ul(self): - before = ( - '
    \n\n' - "
  • No nl's between the ul and the li.
    • \n\n" - "
  • No nl's between li's.\n\n" - 'But there should be two before this line.
    • \n\n' - '
' - ) + @patch('olympia.amo.templatetags.jinja_helpers.urlresolvers.get_outgoing_url') + def test_link_markdown(self, mock_get_outgoing_url): + mock_get_outgoing_url.return_value = 'https://www.mozilla.org' + before = 'Heres a link [to here!](https://www.mozilla.org "Click me!")' after = ( - '
    ' - "
  • No nl's between the ul and the li.
    • " - "
  • No nl's between li's.\n\n" - 'But there should be two before this line.
    • ' - '
' + 'Heres a link ' + '' + 'to here!' + '' ) assert self.newlines_helper(before) == after - def test_newlines_ul_tight(self): + def test_abbr_markdown(self): before = ( - 'There should be one nl between this and the ul.\n' - '
  • test
  • test
\n' - "There should be no nl's above this line." + 'TGIF ROFL\n\n' + '*[TGIF]:i stand for this\n\n' + '*[ROFL]: i stand for that\n\n' ) - after = ( - 'There should be one nl between this and the ul.\n' - '
  • test
  • test
' - "There should be no nl's above this line." + 'TGIF ' + 'ROFL' ) assert self.newlines_helper(before) == after - def test_newlines_ul_loose(self): - before = ( - "There should be two nl's between this and the ul.\n\n" - '
  • test
  • test
\n\n' - 'There should be one nl above this line.' - ) - + def test_bold_markdown(self): + before = "Line.\n\n__This line is bold.__\n\n**So is this.**\n\nThis isn't." after = ( - "There should be two nl's between this and the ul.\n\n" - '
  • test
  • test
\n' - 'There should be one nl above this line.' + 'Line.\n\nThis line is bold.\n\n' + "So is this.\n\nThis isn't." ) assert self.newlines_helper(before) == after - def test_newlines_blockquote_tight(self): - before = ( - 'There should be one nl below this.\n' - '
Hi
\n' - "There should be no nl's above this." - ) - + def test_italics_markdown(self): + before = "Line.\n\n_This line is emphasized._\n\n*So is this.*\n\nThis isn't." after = ( - 'There should be one nl below this.\n' - '
Hi
' - "There should be no nl's above this." + 'Line.\n\nThis line is emphasized.\n\n' + "So is this.\n\nThis isn't." ) assert self.newlines_helper(before) == after - def test_newlines_blockquote_loose(self): - before = ( - 'There should be two nls below this.\n\n' - '
Hi
\n\n' - 'There should be one nl above this.' - ) + def test_empty_markdown(self): + before = 'This is a **** test!' + after = before - after = ( - 'There should be two nls below this.\n\n' - '
Hi
\n' - 'There should be one nl above this.' - ) + assert self.newlines_helper(before) == after + + def test_nested_newline(self): + # Nested newlines escape the markdown. + before = '**\nThis line is not bold.\n\n*This is italic***' + after = '**\nThis line is not bold.\n\nThis is italic**' assert self.newlines_helper(before) == after - def test_newlines_inline(self): + def test_code_markdown(self): before = ( - 'If we end a paragraph w/ a non-block-level tag\n\n' - 'The newlines should be kept' + '````' + 'System.out.println("Hello, World!")' + '````\n\n' + ' System.out.println("Hello Again!")' ) - after = before # Should stay the same + after = ( + 'System.out.println("Hello, World!")\n\n' + 'System.out.println("Hello Again!")\n' + ) assert self.newlines_helper(before) == after - def test_newlines_code_inline(self): - before = "Code tags aren't blocks.\n\n" 'alert(test);\n\nSee?' - - after = before # Should stay the same + def test_blockquote_markdown(self): + before = 'Test.\n\n' '> \n' '> - \n' '\ntest.' + after = 'Test.\n
\ntest.' assert self.newlines_helper(before) == after - def test_newlines_li_newlines(self): - before = '
  • \nxx
' + def test_ul_markdown(self): + before = '* \nxx' after = '
  • xx
' assert self.newlines_helper(before) == after - before = '
  • xx\n
' + before = '* xx' after = '
  • xx
' assert self.newlines_helper(before) == after - before = '
  • xx\nxx
' + before = '* xx\nxx' after = '
  • xx\nxx
' assert self.newlines_helper(before) == after - before = '
' - after = '
' + before = '*' + after = before # Doesn't do anything on its own assert self.newlines_helper(before) == after # All together now - before = '
  • \nxx
  • xx\n
  • xx\nxx
  • \n
' - - after = '
  • xx
  • xx
  • xx\nxx
' - - assert self.newlines_helper(before) == after - - def test_newlines_empty_tag(self): - before = 'This is a test!' - after = before - - assert self.newlines_helper(before) == after - - def test_newlines_empty_tag_nested(self): - before = 'This is a test!' - after = before - - assert self.newlines_helper(before) == after - - def test_newlines_empty_tag_block_nested(self): - b = 'Test.\n\n
\ntest.' - a = 'Test.\n\n
test.' - - assert self.newlines_helper(b) == a - - def test_newlines_empty_tag_block_nested_spaced(self): - before = ( - 'Test.\n\n
\n\n
    \n\n
  • ' - '
    • \n\n
\n\n
\ntest.' - ) - after = 'Test.\n\n
test.' + before = '* \nxx\n' '* xx\n' '* \n' '* xx\nxx\n' + after = '
  • xx
  • xx
  • xx\nxx
' assert self.newlines_helper(before) == after - def test_newlines_li_newlines_inline(self): - before = ( - '
  • \ntest\ntest\n\ntest\n
    • ' - '
  • Test test test.
' - ) - - after = ( - '
  • test\ntest\n\ntest
    • ' - '
  • Test test test.
' - ) - + def test_ol_markdown(self): + before = '1. \nxx' + after = '
  1. xx
' assert self.newlines_helper(before) == after - def test_newlines_li_all_inline(self): - before = ( - 'Test with no newlines and block level ' - 'stuff to see what happens.' - ) - - after = before # Should stay the same - + before = '1. xx' + after = '
  1. xx
' assert self.newlines_helper(before) == after - def test_newlines_spaced_blocks(self): - before = ( - '
\n\n
    \n\n
  • \n\ntest\n\n
    • \n\n
\n\n
' - ) - - after = '
  • test
' - + before = '1. xx\nxx' + after = '
  1. xx\nxx
' assert self.newlines_helper(before) == after - def test_newlines_spaced_inline(self): - before = "Line.\n\n\nThis line is bold.\n\n\nThis isn't." - after = before - + before = '1.' + after = before # Doesn't do anything on its own assert self.newlines_helper(before) == after - def test_newlines_nested_inline(self): - before = '\nThis line is bold.\n\nThis is also italic' - after = before + # All together now + before = '1. \nxx\n' '2. xx\n' '3. \n' '4. xx\nxx\n' + after = '
  1. xx
  2. xx
  4. xx\nxx
' assert self.newlines_helper(before) == after def test_newlines_xss_script(self): @@ -1176,7 +1118,7 @@ def test_newlines_xss_script(self): def test_newlines_xss_inline(self): before = 'test' - after = 'test' + after = '<b onclick="alert(\'test\');">test</b>' assert self.newlines_helper(before) == after @@ -1189,45 +1131,17 @@ def test_newlines_attribute_link_doublequote(self, mock_get_outgoing_url): assert 'rel="nofollow"' in parsed - def test_newlines_attribute_singlequote(self): - before = "lol" - after = 'lol' - - assert self.newlines_helper(before) == after - - def test_newlines_attribute_doublequote(self): - before = 'lol' - after = before - - assert self.newlines_helper(before) == after - - def test_newlines_attribute_nestedquotes_doublesingle(self): - before = 'lol' - after = before + def test_newlines_tag(self): + # user-inputted HTML is cleaned and ignored in favour of markdown. + # Disallowed markdown is stripped from the final result. + before = 'This is a bold **test!** \n\n --- \n\n' + after = 'This is a <b>bold</b> test!' assert self.newlines_helper(before) == after - def test_newlines_attribute_nestedquotes_singledouble(self): - before = 'lol' - after = before - - assert self.newlines_helper(before) == after - - def test_newlines_unclosed_b(self): + def test_newlines_unclosed_tag(self): before = 'test' - after = 'test' - - assert self.newlines_helper(before) == after - - def test_newlines_unclosed_b_wrapped(self): - before = 'This is a test' - after = 'This is a test' - - assert self.newlines_helper(before) == after - - def test_newlines_unclosed_li(self): - before = '
  • test
' - after = '
  • test
' + after = '<b>test' assert self.newlines_helper(before) == after diff --git a/src/olympia/devhub/templates/devhub/addons/edit/describe.html b/src/olympia/devhub/templates/devhub/addons/edit/describe.html index 284c4e3f3872..7a795d9b3f96 100644 --- a/src/olympia/devhub/templates/devhub/addons/edit/describe.html +++ b/src/olympia/devhub/templates/devhub/addons/edit/describe.html @@ -1,4 +1,4 @@ -{% from "devhub/includes/macros.html" import tip, empty_unless, flags, select_cats, some_html_tip, trans_readonly %} +{% from "devhub/includes/macros.html" import tip, empty_unless, flags, select_cats, markdown_tip, trans_readonly %}
- {{ some_html_tip() }} + {{ markdown_tip() }} {% else %} {% call empty_unless(addon.description) %}
diff --git a/src/olympia/devhub/templates/devhub/addons/edit/technical.html b/src/olympia/devhub/templates/devhub/addons/edit/technical.html index 887b97eaac3e..8873ba535a74 100644 --- a/src/olympia/devhub/templates/devhub/addons/edit/technical.html +++ b/src/olympia/devhub/templates/devhub/addons/edit/technical.html @@ -1,4 +1,4 @@ -{% from "devhub/includes/macros.html" import tip, some_html_tip, empty_unless, flags %} +{% from "devhub/includes/macros.html" import tip, markdown_tip, empty_unless, flags %} @@ -32,10 +32,10 @@

{% if editable %} {{ main_form.developer_comments }} {{ main_form.developer_comments.errors }} - {{ some_html_tip() }} + {{ markdown_tip() }} {% else %} {% call empty_unless(addon.developer_comments) %} -
{{ addon|all_locales('developer_comments') }}
+
{{ addon|all_locales('developer_comments', nl2br=True) }}
{% endcall %} {% endif %} diff --git a/src/olympia/devhub/templates/devhub/addons/submit/describe.html b/src/olympia/devhub/templates/devhub/addons/submit/describe.html index 969280bb0e92..ba80a5b51454 100644 --- a/src/olympia/devhub/templates/devhub/addons/submit/describe.html +++ b/src/olympia/devhub/templates/devhub/addons/submit/describe.html @@ -1,4 +1,4 @@ -{% from "devhub/includes/macros.html" import some_html_tip, select_cats %} +{% from "devhub/includes/macros.html" import markdown_tip, select_cats %} {% from "includes/forms.html" import tip %} {% extends "devhub/addons/submit/base.html" %} @@ -113,7 +113,7 @@

{{ _('Describe Add-on') }}

data-for-startswith="{{ describe_form.description.auto_id }}_" data-minlength="{{ describe_form.description.field.min_length }}">
- {{ some_html_tip() }} + {{ markdown_tip() }} {% endif %} {% if addon.type != amo.ADDON_STATICTHEME %} @@ -180,7 +180,7 @@

{{ _('Describe Add-on') }}

{{ license_form.text.errors }} {{ license_form.text.label_tag() }} {{ license_form.text }} - {{ some_html_tip() }} + {{ markdown_tip() }} {% endif %} diff --git a/src/olympia/devhub/templates/devhub/includes/macros.html b/src/olympia/devhub/templates/devhub/includes/macros.html index 3ce176200905..7493132106b7 100644 --- a/src/olympia/devhub/templates/devhub/includes/macros.html +++ b/src/olympia/devhub/templates/devhub/includes/macros.html @@ -1,7 +1,7 @@ {% extends "includes/forms.html" %} {% macro some_html_tip(title=None) %} -

+

{# L10n: %s is a list of HTML tags. #} +{# L10n: %s is a list of markdown syntax. #} +{{ _('Some Markdown supported.') }} +

+{% endmacro %} + + {% macro empty_unless(truthy) %} {% if truthy %} {{ caller() }} diff --git a/src/olympia/devhub/templates/devhub/includes/policy_form.html b/src/olympia/devhub/templates/devhub/includes/policy_form.html index 6018405858c2..96dc841b321a 100644 --- a/src/olympia/devhub/templates/devhub/includes/policy_form.html +++ b/src/olympia/devhub/templates/devhub/includes/policy_form.html @@ -1,4 +1,4 @@ -{% from "devhub/includes/macros.html" import tip, some_html_tip %} +{% from "devhub/includes/macros.html" import tip, markdown_tip %} {{ tip(_('End-User License Agreement'), _('Please note that a EULA is not ' @@ -13,7 +13,7 @@ {{ policy_form.eula.label }} {{ policy_form.eula }} - {{ some_html_tip() }} + {{ markdown_tip() }} @@ -31,7 +31,7 @@ {{ policy_form.privacy_policy.label }} {{ policy_form.privacy_policy }} - {{ some_html_tip() }} + {{ markdown_tip() }} diff --git a/src/olympia/devhub/tests/test_views_edit.py b/src/olympia/devhub/tests/test_views_edit.py index 4fb1ec60d5c1..853d816cffec 100644 --- a/src/olympia/devhub/tests/test_views_edit.py +++ b/src/olympia/devhub/tests/test_views_edit.py @@ -353,14 +353,14 @@ def test_edit_xss(self): Let's try to put xss in our description, and safe html, and verify that we are playing safe. """ - self.addon.description = 'This\nIS' "" + self.addon.description = 'This\n**IS**' "" self.addon.save() response = self.client.get(self.url) assert response.status_code == 200 doc = pq(response.content) assert doc('#addon-description span[lang]').html() == ( - "This
IS<script>alert('awesome')</script>" + "This
IS<script>alert('awesome')</script>" ) response = self.client.get(self.describe_edit_url) @@ -368,7 +368,7 @@ def test_edit_xss(self): assert b'