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.
EntityForm has a magic setter and getter to take care of triggering a deprecation error, eg:
public function __get($name) {
// Removing core's usage of ::setEntityManager means that this deprecated
// service wont be set. We provide it here for backwards compatibility.
if ($name === 'entityManager') {
@trigger_error('EntityForm::entityManager is deprecated in drupal:8.0.0 and is removed from drupal:9.0.0. Use EntityForm::entityTypeManager instead. See https://www.drupal.org/node/2549139', E_USER_DEPRECATED);
return $this->privateEntityManager ?: \Drupal::entityManager();
}
}
However, this does nothing when $name is not entityManager. So any form class that does $this->foo = 'bar' without having declared $foo as a class property will suddenly find that setting the value has no effect.
Credit to @aalin to figuring this one at #3093329: $moduleEntityData error in ComponentSectionForm.php.
Comment | File | Size | Author |
---|---|---|---|
#21 | 20-21-interdiff.txt | 1.04 KB | alexpott |
#21 | 3094913-21.patch | 1.41 KB | alexpott |
#20 | 3094913-21.patch | 1.87 KB | alexpott |
#20 | 3094913-21.test-only.patch | 719 bytes | alexpott |
#20 | 15-21-interdiff.txt | 1020 bytes | alexpott |
Comments
Comment #2
joachim CreditAttribution: joachim as a volunteer commentedComment #3
joachim CreditAttribution: joachim as a volunteer commentedComment #4
joelpittetHow about something like this?
Comment #5
joelpittetAdding follow-up from parent
Comment #6
joachim CreditAttribution: joachim as a volunteer commentedI don't understand we need the intermediary of the privateData array.
Can we not just do
$this->{$name}
?Comment #7
BerdirYes, I think we can just check for/set $this->$name. There's also a typo to fix in __set(): "This *is* property".
Comment #8
neel24 CreditAttribution: neel24 at Google Code-In commentedI made some changes based on comment #7. Attaching an interdiff as well.
Comment #9
agrochal CreditAttribution: agrochal at Google Code-In commentedIn the "__set" function, there's still
$this->privateData[$name] = value;
. I think we can also remove declaration of this arrayprivate $privateData = [];
.Comment #10
Hardik_Patel_12 CreditAttribution: Hardik_Patel_12 at QED42 commentedChanges based on comment #4 , #7 and #9. Attaching an interdiff as well.
Comment #11
joachim CreditAttribution: joachim as a volunteer commentedLooking good, just the comment needs work:
This sentence doesn't read properly.
Comment #12
Madhura BK CreditAttribution: Madhura BK at UniMity Solutions Pvt Limited for Drupal India Association commentedComment #13
AkashKumar07 CreditAttribution: AkashKumar07 at OpenSense Labs commentedRemoved declaration of
Comment #14
Hardik_Patel_12 CreditAttribution: Hardik_Patel_12 at QED42 commented@AkashkumarOSL ,
Kindly look comment #11 and #10 , we need to change comments only now in set method.
Comment #15
neel24 CreditAttribution: neel24 at Google Code-In commentedI've created a new patch based on the necessary changes.
Comment #16
joachim CreditAttribution: joachim as a volunteer commentedLGTM.
Thanks!
Comment #17
AkashKumar07 CreditAttribution: AkashKumar07 at OpenSense Labs commentedPlease review this patch.
Comment #18
Hardik_Patel_12 CreditAttribution: Hardik_Patel_12 at QED42 commentedPatch written in #15 looks great to me.
@AkashkumarOSL , changes done in #17's patch is not required kindly look patch written in #15.
Comment #19
alexpottYep #17 is an incorrect bugfix. #15 looks like the correct fix but #17 shows that we don't have any test coverage. We should have that.
Comment #20
alexpottActually there was a bug in #15 too. Here's a test and fix.
Comment #21
alexpottThis is not necessary.
Comment #23
BerdirLooks good.
Note: This BC layer was already removed in 9.0.x, so this does not need to be committed there.
Comment #26
catchCommitted 7d7c475 and pushed to 8.9.x, then cherry-picked to 8.8.x. Thanks!