[wp-trac] [WordPress Trac] #58317: Refactor determine_locale() for performance

[WordPress Trac]

#58317: Refactor determine_locale() for performance
--------------------------------------+--------------------------
 Reporter:  Cybr                      |       Owner:  swissspidy
     Type:  enhancement               |      Status:  reopened
 Priority:  normal                    |   Milestone:  6.3
Component:  I18N                      |     Version:  trunk
 Severity:  normal                    |  Resolution:
 Keywords:  has-patch has-unit-tests  |     Focuses:  performance
--------------------------------------+--------------------------
Changes (by Cybr):

 * status:  closed => reopened
 * resolution:  fixed =>


Comment:

 Hi @swissspidy, thank you for patching the improvements, but I've noticed
 that most of the suggestions I've made haven't been fully considered.

 Admin: https://3v4l.org/suK5D (my proposed patch is 40% faster)
 Front: https://3v4l.org/JM0l4 (my proposed patch is 5% faster)

 I'd like to discuss a few specific points:

 1. `is_admin()` is more often `true` than the insecure test of
 `wp_is_json_request()`; hence I meticulously switched those checks around.
 2. `else if` vs `elseif`... WordPress has no standard on which to use. I
 suggest using `elseif` because one large word is easier to read than two
 separate words: https://nceo.umn.edu/docs/presentations/nceo-lep-iep-
 ascdhandoutchunking.pdf.
 3. Many developers have been advocating for the use of short ternary
 operators, but after [https://make.wordpress.org/core/2019/07/12/php-
 coding-standards-changes/ 4 years of requesting comments], there's
 absolutely no data given on why short ternary is "not readable." All we
 got close to receiving were dinosaur excuses and that characters of the
 same length look alike (how can one see all six?). The patch as-committed
 adds extra jumps and operations without sufficient justification.
 4. `! empty( $_GET['wp_lang'] )` is checked twice now in the same clause.
 5. **The patch as-committed introduces a bug**, whereas my proposal did
 not: if a bogus Unicode character is sent like `?wp_lang=%E1%A0%8E` (add
 unit tests?), we'll have no language returned at all. To avoid this issue,
 we should still call `get_{user_}locale()` even if the query or cookie is
 set. We could account for that without using short ternary, but that is 5%
 slower than simply writing the `get_{user_}locale()`-output in most cases
 (https://3v4l.org/CD9MQ).

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