Closed (won't fix)
Project:
HTML2Text
Version:
master
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Reporter:
Created:
14 May 2008 at 01:34 UTC
Updated:
25 May 2012 at 01:55 UTC
Jump to comment: Most recent file
<strong>, <em> etc can all have other attributes. The regexps in drupal_html_to_text() don't handle this. Attached patches for D6 and the D5 backport in contrib (http://drupal.org/project/html_to_text)
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 258192_drupal_html_to_text_strong_with_attrs.2.d6.patch | 873 bytes | dww |
| #2 | 258192_drupal_html_to_text_strong_with_attrs.2.d5.patch | 860 bytes | dww |
| #1 | 258192_drupal_html_to_text_strong_with_attrs.d6.patch | 865 bytes | dww |
| #1 | 258192_drupal_html_to_text_strong_with_attrs.d5.patch | 852 bytes | dww |
| drupal_html_to_text_strong_with_attrs.d5.patch | 862 bytes | dww |
Comments
Comment #1
dwwWhoops, that was a little too greedy. We need to ensure the rest of the goo is something after at least 1 space, so that we don't accidentally match other tags that start with those names (e.g. confusing
<blockquote>with<b>). This is better.Comment #2
dwwIn IRC, pointed out that the
' +'part I added in #1 to match 1 or more spaces can confuse regexp engines since a space is also matched by[^>]. It all works fine, but I guess it can potentially be slower this way. Atomic grouping to the rescue.Tested and it still works. I see absolutely no change in speed, but whatever, I'm not carefully benchmarking any of this. I suppose making the regexp a little more complicated and harder to read is no reason not to optimize something that's not performance intensive in the first place. *smirk*
Comment #3
chx commentedYeah, it's fine.
Comment #4
gábor hojtsyCommitted to 6.x. I guess this needs to move to Drupal 5 and 7 as well. Moving to 5 first.
Comment #5
dwwThanks! Actually, once it moves to 5.x, it needs to switch projects, so let's get this in 7.x core next. chx can always commit this to 5.x contrib now that he's seen it and seen it go into 6.x. ;)
Comment #6
dww(Oh, and I should note that http://drupal.org/files/issues/258192_drupal_html_to_text_strong_with_at... still applies cleanly to HEAD without offsets).
Comment #7
dries commentedCommitted to CVS HEAD.
Comment #8
drumm"once it moves to 5.x, it needs to switch projects"
Comment #9
dww@drumm: Sadly, chx disabled the issue queue on the real html_to_text project: http://drupal.org/project/html_to_text. You just moved it here: http://drupal.org/project/html2txt -- which is some old abandoned thing. :(
@chx: this is exactly why it's a pain in the ass not to have the issue queue enabled for your module. Can you please reconsider your decision and re-enable it so there's a place for backport patches like this? If people submit to your queue and you need to bump something to the core queue, that's easy (I end up doing that for update_status vs. update when necessary). While you're thinking about this, can you also either commit the patch and/or give me CVS access so I can take care of obvious stuff like this? ;)
Thanks,
-Derek
Comment #10
robert castelo commented