[wp-trac] [WordPress Trac] #47514: Change priority of make_clickable callback to boost performance

WordPress Trac noreply at wordpress.org
Sun Sep 22 05:56:57 UTC 2024


#47514: Change priority of make_clickable callback to boost performance
-------------------------+------------------------------
 Reporter:  olliverh87   |       Owner:  (none)
     Type:  enhancement  |      Status:  new
 Priority:  normal       |   Milestone:  Awaiting Review
Component:  Comments     |     Version:  5.2.1
 Severity:  minor        |  Resolution:
 Keywords:               |     Focuses:  performance
-------------------------+------------------------------

Comment (by dmsnell):

 @SergeyBiryukov interestingly, both outputs you listed from
 `force_balance_tags()` changes the markup to a semantically-equivalent
 document. with and without the patches, the generated DOM is identical.
 They are both wrong, but they are both wrong in the same way, and both
 //produce invalid markup//.

 Normalized output without the patch.
 {{{
 <p><a href="#" rel="nofollow">test</a></p><a href="#" rel="nofollow">
 </a><p><a href="#" rel="nofollow"> </a><a href="http://ftp.x"
 rel="nofollow">http://ftp.x</a></p>
 }}}

 Normalized output with the patch.
 {{{
 <p><a href="#" rel="nofollow">test</a></p><a href="#" rel="nofollow">
 </a><p><a href="#" rel="nofollow"> </a><a href="http://ftp.x"
 rel="nofollow">http://ftp.x</a></p>
 }}}

 The correct normalization would be as follows.
 {{{
 <a href="#" rel="nofollow">test

  ftp.x</a>
 }}}

 We can see from the invalid behaviors that a link is generated with the
 wrong URL. It's "invalid" to open a new `A` when one is already open, but
 the result is that it automatically closes any previously-open `A`
 elements.

 It seems like the real culprit is `make_clickable()`, which isn't aware of
 when it's already inside a link. `wpautop()` doesn't help (it's what
 creates the invalid HTML in your examples), but maybe this would help if
 we simply prevented `make_clickable()` from creating a link _from_
 existing link content. It //tries// to do this, but in the same naive way
 it tries to parse HTML.

 I was exploring code impacted by a refactor in `force_balance_tags()` to
 use the HTML API, but in this case we need to address several of these
 functions. I believe that the performance issues will be addressed though,
 because the HTML API was built for cases like these.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/47514#comment:4>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform


More information about the wp-trac mailing list