Respect CSS precedence rules in HTMLRuleset (#4926)

Fixes #4527

CHANGELOG.txt

@@ -10,6 +10,7 @@ Changelog
  * Improved diffing of StreamFields when comparing page revisions (Karl Hobley)
  * Highlight broken links to pages and missing documents in rich text (Brady Moe)
  * Preserve links when copy-pasting rich text content from Wagtail to other tools (Thibaud Colas)
+ * Rich text to contentstate conversion now prioritises more specific rules, to accommodate `<p>` and `<br>` elements with attributes (Matt Westcott)
  * Fix: Set `SERVER_PORT` to 443 in `Page.dummy_request()` for HTTPS sites (Sergey Fedoseev)
  * Fix: Include port number in `Host` header of `Page.dummy_request()` (Sergey Fedoseev)
  * Fix: Validation error messages in `InlinePanel` no longer count towards `max_num` when disabling the 'add' button (Todd Dembrey, Thibaud Colas)

docs/releases/2.5.rst

@@ -20,6 +20,7 @@ Other features
  * Improved diffing of StreamFields when comparing page revisions (Karl Hobley)
  * Highlight broken links to pages and missing documents in rich text (Brady Moe)
  * Preserve links when copy-pasting rich text content from Wagtail to other tools (Thibaud Colas)
+ * Rich text to contentstate conversion now prioritises more specific rules, to accommodate ``<p>`` and ``<br>`` elements with attributes (Matt Westcott)
 
 
 Bug fixes

wagtail/admin/rich_text/converters/html_ruleset.py

@@ -19,7 +19,7 @@ class HTMLRuleset():
     'a[linktype="page"]' = matches any <a> element with a 'linktype' attribute equal to 'page'
     """
     def __init__(self, rules=None):
-        # mapping of element name to a list of (attr_check, result) tuples
+        # mapping of element name to a sorted list of (precedence, attr_check, result) tuples
         # where attr_check is a callable that takes an attr dict and returns True if they match
         self.element_rules = {}
 
@@ -36,22 +36,28 @@ class HTMLRuleset():
 
     def _add_element_rule(self, name, result):
         # add a rule that matches on any element with name `name`
-        self.element_rules.setdefault(name, []).append(
+        rules = self.element_rules.setdefault(name, [])
-            ((lambda attrs: True), result)
+        # element-only rules have priority 2 (lower)
-        )
+        rules.append((2, (lambda attrs: True), result))
+        # sort list on priority
+        rules.sort(key=lambda t: t[0])
 
     def _add_element_with_attr_rule(self, name, attr, result):
         # add a rule that matches any element with name `name` which has the attribute `attr`
-        self.element_rules.setdefault(name, []).append(
+        rules = self.element_rules.setdefault(name, [])
-            ((lambda attrs: attr in attrs), result)
+        # element-and-attr rules have priority 1 (higher)
-        )
+        rules.append((1, (lambda attrs: attr in attrs), result))
+        # sort list on priority
+        rules.sort(key=lambda t: t[0])
 
     def _add_element_with_attr_exact_rule(self, name, attr, value, result):
         # add a rule that matches any element with name `name` which has an
         # attribute `attr` equal to `value`
-        self.element_rules.setdefault(name, []).append(
+        rules = self.element_rules.setdefault(name, [])
-            ((lambda attrs: attr in attrs and attrs[attr] == value), result)
+        # element-and-attr rules have priority 1 (higher)
-        )
+        rules.append((1, (lambda attrs: attr in attrs and attrs[attr] == value), result))
+        # sort list on priority
+        rules.sort(key=lambda t: t[0])
 
     def add_rule(self, selector, result):
         match = ELEMENT_SELECTOR.match(selector)
@@ -88,6 +94,6 @@ class HTMLRuleset():
         except KeyError:
             return None
 
-        for attr_check, result in rules_to_test:
+        for precedence, attr_check, result in rules_to_test:
             if attr_check(attrs):
                 return result

wagtail/admin/tests/test_contentstate.py

@@ -787,3 +787,22 @@ class TestHtmlToContentState(TestCase):
                 {'inlineStyleRanges': [], 'text': 'After', 'depth': 0, 'type': 'unstyled', 'key': '00000', 'entityRanges': []},
             ]
         })
+
+    def test_p_with_class(self):
+        # Test support for custom conversion rules which require correct treatment of
+        # CSS precedence in HTMLRuleset. Here, <p class="intro"> should match the
+        # 'p[class="intro"]' rule rather than 'p' and thus become an 'intro-paragraph' block
+        converter = ContentstateConverter(features=['intro'])
+        result = json.loads(converter.from_database_format(
+            '''
+            <p class="intro">before</p>
+            <p>after</p>
+            '''
+        ))
+        self.assertContentStateEqual(result, {
+            'blocks': [
+                {'key': '00000', 'inlineStyleRanges': [], 'entityRanges': [], 'depth': 0, 'text': 'before', 'type': 'intro-paragraph'},
+                {'key': '00000', 'inlineStyleRanges': [], 'entityRanges': [], 'depth': 0, 'text': 'after', 'type': 'unstyled'}
+            ],
+            'entityMap': {}
+        })

wagtail/admin/tests/test_html_ruleset.py

@@ -22,3 +22,11 @@ class TestHTMLRuleset(TestCase):
         self.assertEqual(ruleset.match('a', {'class': 'button', 'linktype': 'page'}), 'page-link')
         self.assertEqual(ruleset.match('a', {'class': 'button', 'linktype': 'silly page'}), 'silly-page-link')
         self.assertEqual(ruleset.match('a', {'class': 'button', 'linktype': 'sensible page'}), 'sensible-page-link')
+
+    def test_precedence(self):
+        ruleset = HTMLRuleset()
+        ruleset.add_rule('p', 'normal-paragraph')
+        ruleset.add_rule('p[class="intro"]', 'intro-paragraph')
+        ruleset.add_rule('p', 'normal-paragraph-again')
+
+        self.assertEqual(ruleset.match('p', {'class': 'intro'}), 'intro-paragraph')

wagtail/tests/testapp/wagtail_hooks.py

@@ -6,6 +6,7 @@ import wagtail.admin.rich_text.editors.draftail.features as draftail_features
 from wagtail.admin.action_menu import ActionMenuItem
 from wagtail.admin.menu import MenuItem
 from wagtail.admin.rich_text import HalloPlugin
+from wagtail.admin.rich_text.converters.html_to_contentstate import BlockElementHandler
 from wagtail.admin.search import SearchArea
 from wagtail.core import hooks
 
@@ -107,6 +108,20 @@ def register_blockquote_feature(features):
     )
 
 
+# register 'intro' as a rich text feature which converts an `intro-paragraph` contentstate block
+# to a <p class="intro"> tag in db HTML and vice versa
+@hooks.register('register_rich_text_features')
+def register_intro_rule(features):
+    features.register_converter_rule('contentstate', 'intro', {
+        'from_database_format': {
+            'p[class="intro"]': BlockElementHandler('intro-paragraph'),
+        },
+        'to_database_format': {
+            'block_map': {'intro-paragraph': {'element': 'p', 'props': {'class': 'intro'}}},
+        }
+    })
+
+
 class PanicMenuItem(ActionMenuItem):
     label = "Panic!"
     name = 'action-panic'