From af29bf7bc92c291e50f7b7a7c08b713c1f3214b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Tue, 22 Oct 2013 15:50:37 +0200 Subject: [PATCH 1/5] [FIX] tools: mail: fixed shortening of html content. Fixed length computation of text in html nodes: multiples successive whitespaces are considered as one whitespaces; better truncate position when adding a read more link; now always protect words (placed after the first word that exceeds the shorten position); pre nodes are preserved about whitespaces; when the read more link should go into a quote, it instead goes at the end of the first parent node not being quoted instead of at a wrong position. Added tests for shorten position. bzr revid: tde@openerp.com-20131022135037-igauu2kkglvdrqu7 --- openerp/tests/test_mail.py | 55 +++++++++++++++++++++++++++- openerp/tools/mail.py | 74 +++++++++++++++++++++++++------------- 2 files changed, 104 insertions(+), 25 deletions(-) diff --git a/openerp/tests/test_mail.py b/openerp/tests/test_mail.py index c8bbc0c862b..df1b5e15a11 100755 --- a/openerp/tests/test_mail.py +++ b/openerp/tests/test_mail.py @@ -149,6 +149,59 @@ class TestCleaner(unittest2.TestCase): for text in out_lst: self.assertNotIn(text, new_html, 'html_email_cleaner did not remove unwanted content') + def test_05_shorten(self): + # TEST: shorten length + test_str = '''
+ + +

Hello, Raoul + You are + pretty

+Really +
+''' + # shorten at 'H' of Hello -> should shorten after Hello, + html = html_email_clean(test_str, shorten=True, max_length=1, remove=True) + self.assertIn('Hello,', html, 'html_email_cleaner: shorten error or too short') + self.assertNotIn('Raoul', html, 'html_email_cleaner: shorten error or too long') + self.assertIn('read more', html, 'html_email_cleaner: shorten error about read more inclusion') + # shorten at 'are' -> should shorten after are + html = html_email_clean(test_str, shorten=True, max_length=17, remove=True) + self.assertIn('Hello,', html, 'html_email_cleaner: shorten error or too short') + self.assertIn('Raoul', html, 'html_email_cleaner: shorten error or too short') + self.assertIn('are', html, 'html_email_cleaner: shorten error or too short') + self.assertNotIn('pretty', html, 'html_email_cleaner: shorten error or too long') + self.assertNotIn('Really', html, 'html_email_cleaner: shorten error or too long') + self.assertIn('read more', html, 'html_email_cleaner: shorten error about read more inclusion') + + # TEST: shorten in quote + test_str = '''
Blahble + bluih blouh +
This is a quote + And this is quite a long quote, after all. +
+
''' + # shorten in the quote + html = html_email_clean(test_str, shorten=True, max_length=25, remove=True) + self.assertIn('Blahble', html, 'html_email_cleaner: shorten error or too short') + self.assertIn('bluih', html, 'html_email_cleaner: shorten error or too short') + self.assertIn('blouh', html, 'html_email_cleaner: shorten error or too short') + self.assertNotIn('quote', html, 'html_email_cleaner: shorten error or too long') + self.assertIn('read more', html, 'html_email_cleaner: shorten error about read more inclusion') + # shorten in second word + html = html_email_clean(test_str, shorten=True, max_length=9, remove=True) + self.assertIn('Blahble', html, 'html_email_cleaner: shorten error or too short') + self.assertIn('bluih', html, 'html_email_cleaner: shorten error or too short') + self.assertNotIn('blouh', html, 'html_email_cleaner: shorten error or too short') + self.assertNotIn('quote', html, 'html_email_cleaner: shorten error or too long') + self.assertIn('read more', html, 'html_email_cleaner: shorten error about read more inclusion') + # shorten waaay too large + html = html_email_clean(test_str, shorten=True, max_length=900, remove=True) + self.assertIn('Blahble', html, 'html_email_cleaner: shorten error or too short') + self.assertIn('bluih', html, 'html_email_cleaner: shorten error or too short') + self.assertIn('blouh', html, 'html_email_cleaner: shorten error or too short') + self.assertNotIn('quote', html, 'html_email_cleaner: shorten error or too long') + def test_10_email_text(self): """ html_email_clean test for text-based emails """ new_html = html_email_clean(test_mail_examples.TEXT_1, remove=True) @@ -230,7 +283,7 @@ class TestCleaner(unittest2.TestCase): for ext in test_mail_examples.BUG_1_OUT: self.assertNotIn(ext, new_html, 'html_email_cleaner did not removed invalid content') - new_html = html_email_clean(test_mail_examples.BUG2, remove=True, shorten=True, max_length=4000) + new_html = html_email_clean(test_mail_examples.BUG2, remove=True, shorten=True, max_length=250) for ext in test_mail_examples.BUG_2_IN: self.assertIn(ext, new_html, 'html_email_cleaner wrongly removed valid content') for ext in test_mail_examples.BUG_2_OUT: diff --git a/openerp/tools/mail.py b/openerp/tools/mail.py index b287ded9d6d..2e84f0b4420 100644 --- a/openerp/tools/mail.py +++ b/openerp/tools/mail.py @@ -175,20 +175,38 @@ def html_email_clean(html, remove=False, shorten=False, max_length=300): iteration += 1 new_node = _insert_new_node(node, -1, new_node_tag, text[idx:] + (cur_node.tail or ''), None, {}) - def _truncate_node(node, position, find_first_blank=True): + def _truncate_node(node, position, simplify_whitespaces=True): + """ Truncate a node text at a given position. This algorithm will shorten + at the end of the word whose ending character exceeds position. + + :param bool simplify_whitespaces: whether to try to count all successive + whitespaces as one character. This + option should not be True when trying + to keep 'pre' consistency. + """ if node.text is None: node.text = '' - # truncate text - end_position = position if len(node.text) >= position else len(node.text) - innertext = node.text[0:end_position] - outertext = node.text[end_position:] - if find_first_blank: - stop_idx = outertext.find(' ') + + if simplify_whitespaces: + cur_char_nbr = 0 + word = None + node_words = node.text.strip(' \t\r\n').split() + for word in node_words: + cur_char_nbr += len(word) + if cur_char_nbr >= position: + break + stop_idx = node.text.find(word) + len(word) if stop_idx == -1: - stop_idx = len(outertext) + stop_idx = len(node.text) else: - stop_idx = 0 - node.text = innertext + outertext[0:stop_idx] + stop_idx = position + stop_idx = stop_idx if len(node.text) >= stop_idx else len(node.text) + + # compose new text bits + innertext = node.text[0:stop_idx] + outertext = node.text[stop_idx:] + node.text = innertext + # create ... read more node read_more_node = _create_node('span', ' ... ', None, {'class': 'oe_mail_expand'}) read_more_link_node = _create_node('a', 'read more', None, {'href': '#', 'class': 'oe_mail_expand'}) @@ -223,17 +241,16 @@ def html_email_clean(html, remove=False, shorten=False, max_length=300): html = '
%s
' % html root = lxml.html.fromstring(html) - # remove all tails and replace them by a span element, because managing text and tails can be a pain in the ass - for node in root.getiterator(): + quote_tags = re.compile(r'(\n(>)+[^\n\r]*)') + signature = re.compile(r'([-]{2,}[\s]?[\r\n]{1,2}[\s\S]+)') + for node in root.iter(): + # remove all tails and replace them by a span element, because managing text and tails can be a pain in the ass if node.tail: tail_node = _create_node('span', node.tail) node.tail = None node.addnext(tail_node) - # form node and tag text-based quotes and signature - quote_tags = re.compile(r'(\n(>)+[^\n\r]*)') - signature = re.compile(r'([-]{2,}[\s]?[\r\n]{1,2}[\s\S]+)') - for node in root.getiterator(): + # form node and tag text-based quotes and signature _tag_matching_regex_in_text(quote_tags, node, 'span', {'text_quote': '1'}) _tag_matching_regex_in_text(signature, node, 'span', {'text_signature': '1'}) @@ -245,7 +262,10 @@ def html_email_clean(html, remove=False, shorten=False, max_length=300): quote_begin = False overlength = False cur_char_nbr = 0 - for node in root.getiterator(): + for node in root.iter(): + # node_text = re.sub('\s{2,}', ' ', node.text and node.text.strip(' \t\r\n') or '') # do not take into account multiple spaces that are displayed as max 1 space in html + node_text = ' '.join((node.text and node.text.strip(' \t\r\n') or '').split()) + # root: try to tag the client used to write the html if 'WordSection1' in node.get('class', '') or 'MsoNormal' in node.get('class', ''): root.set('msoffice', '1') @@ -277,27 +297,30 @@ def html_email_clean(html, remove=False, shorten=False, max_length=300): # 1/ truncate the text at the next available space # 2/ create a 'read more' node, next to current node # 3/ add the truncated text in a new node, next to 'read more' node - if shorten and not overlength and cur_char_nbr + len(node.text or '') > max_length: + if shorten and not overlength and cur_char_nbr + len(node_text) > max_length: node_to_truncate = node while node_to_truncate.get('in_quote') and node_to_truncate.getparent() is not None: node_to_truncate = node_to_truncate.getparent() overlength = True node_to_truncate.set('truncate', '1') - node_to_truncate.set('truncate_position', str(max_length - cur_char_nbr)) - cur_char_nbr += len(node.text or '') + if node_to_truncate == node: + node_to_truncate.set('truncate_position', str(max_length - cur_char_nbr)) + else: + node_to_truncate.set('truncate_position', str(len(node.text or ''))) + cur_char_nbr += len(node_text) # Tree modification # ------------------------------------------------------------ for node in root.iter(): if node.get('truncate'): - _truncate_node(node, int(node.get('truncate_position', '0'))) + _truncate_node(node, int(node.get('truncate_position', '0')), node.tag != 'pre') # Post processing # ------------------------------------------------------------ to_remove = [] - for node in root.getiterator(): + for node in root.iter(): if node.get('in_quote') or node.get('in_overlength'): # copy the node tail into parent text if node.tail and not node.get('tail_remove'): @@ -306,17 +329,20 @@ def html_email_clean(html, remove=False, shorten=False, max_length=300): to_remove.append(node) if node.get('tail_remove'): node.tail = '' + # clean node + for attribute_name in ['in_quote', 'tail_remove', 'in_overlength', 'msoffice', 'hotmail', 'truncate', 'truncate_position']: + node.attrib.pop(attribute_name, None) for node in to_remove: if remove: node.getparent().remove(node) else: if not 'oe_mail_expand' in node.get('class', ''): # trick: read more link should be displayed even if it's in overlength - node_class = node.get('class', '') + ' ' + 'oe_mail_cleaned' + node_class = node.get('class', '') + ' oe_mail_cleaned' node.set('class', node_class) # html: \n that were tail of elements have been encapsulated into -> back to \n html = etree.tostring(root, pretty_print=False) - linebreaks = re.compile(r'([\s]*[\r\n]+[\s]*)<\/span>', re.IGNORECASE | re.DOTALL) + linebreaks = re.compile(r']*>([\s]*[\r\n]+[\s]*)<\/span>', re.IGNORECASE | re.DOTALL) html = _replace_matching_regex(linebreaks, html, '\n') return html From 57c11338ae23c8389da6c4969e897e7b95b782bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Tue, 22 Oct 2013 16:29:08 +0200 Subject: [PATCH 2/5] [IMP] tools: mail: added a protection in a string.find, could have a None argument bzr revid: tde@openerp.com-20131022142908-sol44xaprx1b0b0a --- openerp/tools/mail.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/openerp/tools/mail.py b/openerp/tools/mail.py index 2e84f0b4420..6ab7119236c 100644 --- a/openerp/tools/mail.py +++ b/openerp/tools/mail.py @@ -195,7 +195,10 @@ def html_email_clean(html, remove=False, shorten=False, max_length=300): cur_char_nbr += len(word) if cur_char_nbr >= position: break - stop_idx = node.text.find(word) + len(word) + if word: + stop_idx = node.text.find(word) + len(word) + else: + stop_idx = len(node.text) if stop_idx == -1: stop_idx = len(node.text) else: From 7f61d18ef918ba065179e4893e994841a5f63c97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 23 Oct 2013 14:27:44 +0200 Subject: [PATCH 3/5] [CLEAN] tools: cleaned modified code bzr revid: tde@openerp.com-20131023122744-3b3hayy4f8ss2bjx --- openerp/tools/mail.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/openerp/tools/mail.py b/openerp/tools/mail.py index 6ab7119236c..5530ae4797b 100644 --- a/openerp/tools/mail.py +++ b/openerp/tools/mail.py @@ -187,6 +187,7 @@ def html_email_clean(html, remove=False, shorten=False, max_length=300): if node.text is None: node.text = '' + truncate_idx = -1 if simplify_whitespaces: cur_char_nbr = 0 word = None @@ -196,18 +197,15 @@ def html_email_clean(html, remove=False, shorten=False, max_length=300): if cur_char_nbr >= position: break if word: - stop_idx = node.text.find(word) + len(word) - else: - stop_idx = len(node.text) - if stop_idx == -1: - stop_idx = len(node.text) + truncate_idx = node.text.find(word) + len(word) else: - stop_idx = position - stop_idx = stop_idx if len(node.text) >= stop_idx else len(node.text) + truncate_idx = position + if truncate_idx == -1 or truncate_idx >= len(node.text): + truncate_idx = len(node.text) - 1 # compose new text bits - innertext = node.text[0:stop_idx] - outertext = node.text[stop_idx:] + innertext = node.text[0:truncate_idx] + outertext = node.text[truncate_idx:] node.text = innertext # create ... read more node @@ -215,7 +213,7 @@ def html_email_clean(html, remove=False, shorten=False, max_length=300): read_more_link_node = _create_node('a', 'read more', None, {'href': '#', 'class': 'oe_mail_expand'}) read_more_node.append(read_more_link_node) # create outertext node - overtext_node = _create_node('span', outertext[stop_idx:]) + overtext_node = _create_node('span', outertext[truncate_idx:]) # tag node overtext_node.set('in_overlength', '1') # add newly created nodes in dom From e14de7a63737978de323791e4dbf872e0af6a1c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Wed, 23 Oct 2013 14:38:39 +0200 Subject: [PATCH 4/5] [FIX] Fixed previous commit, car go to len of the string. bzr revid: tde@openerp.com-20131023123839-gvf9ugft2b2xmo9a --- openerp/tools/mail.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openerp/tools/mail.py b/openerp/tools/mail.py index 5530ae4797b..431fb561faf 100644 --- a/openerp/tools/mail.py +++ b/openerp/tools/mail.py @@ -200,8 +200,8 @@ def html_email_clean(html, remove=False, shorten=False, max_length=300): truncate_idx = node.text.find(word) + len(word) else: truncate_idx = position - if truncate_idx == -1 or truncate_idx >= len(node.text): - truncate_idx = len(node.text) - 1 + if truncate_idx == -1 or truncate_idx > len(node.text): + truncate_idx = len(node.text) # compose new text bits innertext = node.text[0:truncate_idx] From dbe253b0e490317dd26c772fd22bb578348a7d80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 24 Oct 2013 10:39:55 +0200 Subject: [PATCH 5/5] [FIX] Fixed bug due to copy-and-paste: outertext is now valid bzr revid: tde@openerp.com-20131024083955-ipbsf8rxbzm0mag3 --- openerp/tools/mail.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openerp/tools/mail.py b/openerp/tools/mail.py index 431fb561faf..a1b0a23dcf5 100644 --- a/openerp/tools/mail.py +++ b/openerp/tools/mail.py @@ -213,7 +213,7 @@ def html_email_clean(html, remove=False, shorten=False, max_length=300): read_more_link_node = _create_node('a', 'read more', None, {'href': '#', 'class': 'oe_mail_expand'}) read_more_node.append(read_more_link_node) # create outertext node - overtext_node = _create_node('span', outertext[truncate_idx:]) + overtext_node = _create_node('span', outertext) # tag node overtext_node.set('in_overlength', '1') # add newly created nodes in dom