[wp-trac] [WordPress Trac] #46886: wp_targeted_link_rel adds the rel attribute when the link has data-target=""

WordPress Trac noreply at wordpress.org
Sat Apr 13 11:44:03 UTC 2019

#46886: wp_targeted_link_rel adds the rel attribute when the link has data-
 Reporter:  jakeparis       |       Owner:  (none)
     Type:  defect (bug)    |      Status:  new
 Priority:  normal          |   Milestone:  Awaiting Review
Component:  Formatting      |     Version:  5.1
 Severity:  normal          |  Resolution:
 Keywords:  has-unit-tests  |     Focuses:

Comment (by birgire):

 For the background story, this was added in #43187.

 By pre-checking for {{{' target'}}} in [attachment:"46886-2.diff"], the
 following will not go to the regex callback:

 <a href="https://example.com" data-target="thisandthat">click here</a>


 but this would:

 <a href="https://example.com" data-target="This is targetting all">click


 One could additionally adjust the regex from:




 to account for the white space in front of the {{{target}}} attribute.
 Here {{{(?:)}}} means a non-capturing group.
 [https://stackoverflow.com/a/15926317/2078474 This] answer by plalx was
 helpful. Tested {{{\s}}} instead of {{{\s+}}} in the changed regex for few
 cases without problem.

 But this will not stop edge cases, where {{{' target='}}} is part of the
 attribute value, like:

 <a href="https://example.com" data-target="This is target=One">click


 from getting the {{{rel}}} part added.

 We note that {{{target=_blank}}}, without {{{"}}} or {{{'}}}, is matched
 by the current regex, so it could make solving the edge case more

 So at minimum, I think we could consider adding the extra space in the
 pre-check, to reduce the false positives.

 Changing an existing regex should be approached with care, but if we go
 that route it would most likely require some extra unit-tests.

 One could also look further into checks within the callback.

Ticket URL: <https://core.trac.wordpress.org/ticket/46886#comment:5>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform

More information about the wp-trac mailing list