Follow up to #269911: Search result trimming should not fall inside HTML entities/tags.

Since PHP 5.0, html_entity_decode() supports utf8 in PHP >= 5.0, so it should be possible to use that instead of decode_entities() in Drupal 7.

CommentFileSizeAuthor
#7 878408-entity-decode-7.patch10.43 KBpwolanin
#5 878408-entity-decode.patch19.55 KBDamien Tournoud
#1 878408-entity-decode-1.patch9.09 KBpwolanin
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Status: Active » Needs review
FileSize
878408-entity-decode-1.patch9.09 KB

Here's a first quick pass. Do we want to maintain a wrapper function?

Dries’s picture

Dries CreditAttribution: Dries commented

That would be lovely. Let's see what the tests will say.

Status: Needs review » Needs work

The last submitted patch, 878408-entity-decode-1.patch, failed testing.

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented

A the failing tests were using the exclude parameter, which the patch proposes to remove.

Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented
Status: Needs work » Needs review
FileSize
878408-entity-decode.patch19.55 KB

I suggest the following: keep the wrapper, as it is not trivial to get the arguments right, and rename it as drupal_html_entity_decode().

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Status: Needs review » Needs work

@DamZ - why break the API more than needed at this point? Let's not rename the function.

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Status: Needs work » Needs review
FileSize
878408-entity-decode-7.patch10.43 KB

This is the same patch as #1 plus removing the irrelevant test case.

Status: Needs review » Needs work

The last submitted patch, 878408-entity-decode-7.patch, failed testing.

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Status: Needs work » Needs review

huh- I find it hard to believe that's a real fail.

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented

#7: 878408-entity-decode-7.patch queued for re-testing.

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented
Status: Needs review » Reviewed & tested by the community

good to go.

Dries’s picture

Dries CreditAttribution: Dries commented

This looks good to go. I might add a "@todo: remove in Drupal 8" but I'm sure we'll remember to do that without the todo.

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD.

Status: Fixed » Closed (fixed)
Issue tags: -Performance

Automatically closed -- issue fixed for 2 weeks with no activity.

Issue tags: +Performance

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.