WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#16432 closed enhancement (fixed)

Add filter to ent2ncr

Reported by: GaryJ Owned by: nacin
Milestone: 3.3 Priority: normal
Severity: minor Version: 3.1
Component: General Keywords: has-patch commit
Focuses: Cc:

Description

There's been several tickets regarding the use of hexadecimal numerical entities instead of decimal numerical entities - it's part of a W3C standard that Content SHOULD use the hexadecimal form of character escapes rather than the decimal form when there are both.

This patch adds hexadecimal entity support to ent2ncr() for the existing named entities. It adds an optional second argument, and defaults to returning the decimal entities so is fully backwards compatible.

Attachments (5)

ent2ncr.patch (20.4 KB) - added by GaryJ 3 years ago.
Adds hexadecimal entity support to ent2ncr()
16432.diff (451 bytes) - added by GaryJ 3 years ago.
Add filter on return value of ent2ncr
16432-alt.diff (457 bytes) - added by GaryJ 3 years ago.
Add filter to allow plugin to short circuit the function
16432-alt2.diff (456 bytes) - added by GaryJ 3 years ago.
Check against null in short circuit.
16432-alt3.diff (460 bytes) - added by GaryJ 3 years ago.
Proper identity check against null in short circuit

Download all attachments as: .zip

Change History (19)

GaryJ3 years ago

Adds hexadecimal entity support to ent2ncr()

comment:1 GaryJ3 years ago

If there's a better performing way to pull out the entities from the $to_ncr array that having to loop through and pull them into a new array first, then please let me know!

I thought about creating two arrays ($to_ncr and $to_hncr), but then figured the sets of named entities may get out of sync. If there's not much change in this function it may be fine to do that.

Alternatively, an ent2hncr() function may be better, which ent2ncr defers to if the 2nd argument is correctly set?

comment:2 westi3 years ago

Should is not Must.

I think a better solution is just to add a filter to this function so a plugin can implement the alternative behaviour.

GaryJ3 years ago

Add filter on return value of ent2ncr

GaryJ3 years ago

Add filter to allow plugin to short circuit the function

comment:3 GaryJ3 years ago

  • Keywords needs-testing added

Two patches - the first is a simple filter on the return value.

The second patch approaches it a slightly different way. The issue with the first, is that for a block of text that is being running through ent2ncr(), all of the string replacements would still be done, even if they weren't then used in the plugin filter function. By letting the plugin short circuit the function (if something truthy is being returned from the plugin filter function, return that from ent2ncr() immediately), it improves performance.

comment:4 follow-up: nacin3 years ago

Short-circuit is fine by me. Let's use null instead.

GaryJ3 years ago

Check against null in short circuit.

comment:5 in reply to: ↑ 4 GaryJ3 years ago

Replying to nacin:

Short-circuit is fine by me. Let's use null instead.

Like so?

comment:6 GaryJ3 years ago

  • Summary changed from Hexadecimal entity support within ent2ncr to Add filter to ent2ncr

Is it too late for this filter to go in to 3.2?

comment:7 nacin3 years ago

  • Keywords needs-testing removed
  • Milestone changed from Awaiting Review to Future Release

Yes. Freeze for enhancements was about 6 weeks ago.

comment:8 nacin3 years ago

By null I meant if ( null !== $filtered = ...

GaryJ3 years ago

Proper identity check against null in short circuit

comment:9 GaryJ3 years ago

Wasn't aware you could do a comparison against an assignment like that - handy! Patched.

comment:10 nacin3 years ago

  • Keywords commit added
  • Milestone changed from Future Release to 3.3

comment:11 follow-up: nacin3 years ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In [18456]:

Add pre_ent2ncr filter. props GaryJ, fixes #16432.

comment:12 in reply to: ↑ 11 westi3 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Replying to nacin:

In [18456]:

Add pre_ent2ncr filter. props GaryJ, fixes #16432.

This change breaks our "no clever code" coding standard. Fixing.

comment:13 westi3 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In [18465]:

Make [18456] more obvious. Fixes #16432.

comment:14 scribu3 years ago

+1 for [18465]. Just because you can doesn't mean you should.

Note: See TracTickets for help on using tickets.