Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#32878 closed defect (bug) (fixed)

More readable regular expressions for Press This URL's

Reported by: dd32 Owned by: dd32
Milestone: 4.4 Priority: normal
Severity: normal Version: 4.2
Component: Press This Keywords: has-patch
Focuses: Cc:


The regular expressions in Press This are hard to read.
The attached patch does (probably among other things)

  • Switches to using ! instead of / as the delimiter, so that / doesn't need escaping inline
  • Removes the escaping of ? and . within character maps [.?] as they don't need escaping there
  • Adds case-insensitive matching to most expressions, aside from /wp-includes/
  • Removes the usage of {1} as a single-character match is implied
  • Not certain what preg_match( '/\/ad[sx]{1}?\//', $src ) was supposed to match (/ad/, /ads/, and /adx/? If so, it's not currently matching /ad/, which this patch changes)

The patch is untested so far, needs to be reviewed to check I didn't miss anything.

Attachments (1)

32878.diff (5.0 KB) - added by dd32 5 years ago.

Download all attachments as: .zip

Change History (5)

5 years ago

#1 @azaozz
5 years ago

The patch looks good. We can also make pretty much all groups non-capturing as they are only used to check if matches exist.

We can probably combine !/([^./]+[-_])?(spinner|loading|spacer|blank)s?([-_][^./]+)?\.[a-z0-9]{3,4}!i and !/(spinner|loading|spacer|blank|rss)\.(gif|jpg|png)!i, as they are pretty close (assuming \.[a-z0-9]{3,4} is to match image extensions).

Last edited 5 years ago by azaozz (previous) (diff)

#2 @kraftbj
5 years ago

  • Version set to 4.2

@azaozz @dd32 - Any other thoughts on this? Anything in particular I can help test that you're more worried about?

#3 @dd32
5 years ago

  • Milestone changed from Awaiting Review to 4.4

#4 @dd32
5 years ago

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

In 34736:

Press This: Make the regular expressions for matching images easier to read by not requiring escaping.
Fixes #32878

Note: See TracTickets for help on using tickets.