Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
TitleResolver service's getTitle() should return consistent type of object. If we look at the getTitle() implementation then we found that there is if and elseif statement in which it returns different type of objects.
Steps to reproduce
In the current route object check the default values by $route->getDefault(), if we have default _title_callback then it returns object of different type but if we have default _title only then it returns object of type TranslatableMarkup
Proposed resolution
Inside the getTitle() of TitleResolver service, for both the cases it should return object of same type.
Comment | File | Size | Author |
---|---|---|---|
#4 | 3310346-4.patch | 766 bytes | Wim Leers |
| |||
#3 | title_resolver_update-3310346-3.patch | 810 bytes | Hemuvyas97 |
#2 | title_resolver_update-3310346-2.patch | 716 bytes | Hemuvyas97 |
Comments
Comment #2
Hemuvyas97 CreditAttribution: Hemuvyas97 as a volunteer commentedComment #3
Hemuvyas97 CreditAttribution: Hemuvyas97 as a volunteer commentedComment #4
Wim LeersInterface docs:
string as a typehint tends to imply that as long as a
Stringable
object is returned, it's fine. But https://www.php.net/manual/en/class.stringable.php only exists since PHP 8, which is why the interface docs indicate juststring
.In Drupal 10, we'll be able to make this better though!
What do you think about this as an alternative? 😊
Comment #5
cilefen CreditAttribution: cilefen commentedWhy is this a support request?
Comment #6
Wim LeersComment #7
smustgrave CreditAttribution: smustgrave at Mobomo commentedMakes sense to me. And my ide will thank you!
Comment #8
Chi CreditAttribution: Chi commentedWill there be any problems if we add native PHP typehints to the method?
Comment #9
Wim Leers@Chi If we don't have to care about PHP 7 support, then we indeed could … if we did a BC break: https://3v4l.org/WYop7
Code in that experiment:
… so AFAICT, we unfortunately can't do that.
Comment #10
Chi CreditAttribution: Chi commented@Wim Leers, aren't BC breaks still acceptable for Drupal 10?
Comment #11
Wim LeersDrupal 10 is in beta, so … no. The API is frozen.
Comment #13
smustgrave CreditAttribution: smustgrave at Mobomo commentedMoving back to RTBC seemed ot be a random failure.
Comment #14
alexpottFixing the issue title to match what we're doing
Comment #15
alexpottCommitted and pushed 6d34fc2b74 to 10.1.x and efeb6f0ec2 to 10.0.x. Thanks!