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

[WordPress Trac]

#46886: wp_targeted_link_rel adds the rel attribute when the link has data-
target=""
----------------------------+------------------------------
 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
 here</a>

 }}}

 One could additionally adjust the regex from:

 {{{
 |<a\s([^>]*target\s*=[^>]*)>|i
 }}}

 to:

 {{{
 |<a\s+((?:[^>]*\s+)?target\s*=[^>]*)>|i
 }}}

 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
 here</a>

 }}}

 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
 involved.


 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