yiisoft / yii

Yii PHP Framework 1.1.x
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
4.84k stars 2.28k forks source link

fix/static method getCanonicalID should not be called dynamically #4544

Closed apphp closed 10 months ago

apphp commented 10 months ago
Q A
Is bugfix? ✔️/❌
New feature? ✔️/❌
Breaks BC? ✔️/❌
Tests pass? ✔️/❌
Fixed issues comma-separated list of tickets # fixed by the PR, if any
what-the-diff[bot] commented 10 months ago

PR Summary

marcovtwout commented 10 months ago

Thanks!

rob006 commented 10 months ago

This change breaks BC: previously if you extend this class and override getCanonicalID(), new implementation was used everywhere. After this change new implementation will be ignored and always CLocale::getCanonicalID() will be used.

marcovtwout commented 10 months ago

@rob006 Thank you for checking in detail, but that is not exactly the case. The method itself was not updated and was already static, and there are more function calls within the framework that were already calling it correctly using ::getCanonicalID()

rob006 commented 10 months ago

@marcovtwout

Before change: https://3v4l.org/ZAIWn After change: https://3v4l.org/bM5Ka

I'm not sure if overwriting getCanonicalID() is even supported, but $this-> could be used here to emulate static:: which was not available in older versions of PHP.

marcovtwout commented 10 months ago

The old behavior (to my surprise) doesn't cause any PHP warning. Looking at the history, it doesn't look like $this was used on purpose, so I would say this change is fairly safe.