Problem/Motivation
Since #3588881: Convert hooks to OOP there is a bug with the "rate_rating" form element.
Preprocess variables such as "type" and others are entirely missing.
This is caused by the change at this place:
https://git.drupalcode.org/project/rate/-/commit/d9cce1e7fa8fc89450073cf...
'form_element__rate_rating' => [
'base hook' => 'form_element',
'initial preprocess' => static::class . ':preprocessFormElementRating',
],
Before #3588881: Convert hooks to OOP went in, there was no "initial preprocess" definition. Now since that definition was added, the original "initial preprocess" definition of the base hook "form_element" is not being called anymore. That's why the essential variables are gone now. The effect is that some classes are now incorrectly set, such as "form-type-radio" is now just "form-type-" (because the "type" variable is now missing).
Steps to reproduce
Update from 3.2.1 to 3.3.0 and see wrong classes set at the .form-item elements.
Proposed resolution
Although calling the FormPreprocess services is not officially supported (it is an @internal class), I don't see any other way than calling it manually from the newly defined "initial preprocess" function. I don't know the reason why it was added inside the scope of refactoring to OOP hooks, but maybe there was a certain reason so I'm not proprosing to remove it (unless maintainer sees no reason at all).
Remaining tasks
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | 3612827-form-element-not-properly-preprocessed.patch | 1.41 KB | mxh |
Issue fork rate-3612827
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3612827-preprocess
changes, plain diff MR !57
- 3612827-rating-form-element
changes, plain diff MR !55
Comments
Comment #3
mxh commentedImplemented the suggested approach in MR55.
Comment #4
mxh commentedHere is the proposed fix as a patch, in case someone needs a quick bandaid.
Comment #5
ivnishLooks like this commit https://git.drupalcode.org/project/rate/-/commit/93fe444461fadfe9928dc47... introduced changes, not OOP hooks
Comment #6
mxh commentedI don't see the change within https://git.drupalcode.org/project/rate/-/commit/93fe444461fadfe9928dc47...
But I see it within https://git.drupalcode.org/project/rate/-/commit/d9cce1e7fa8fc89450073cf...
Also when comparing the contents of rate.module before OOP hooks went in confirms that this "initial preprocess" entry got introduced from OOP hooks.
Comment #8
ivnishI investigated this. Could you test my MR https://git.drupalcode.org/project/rate/-/merge_requests/57/diffs ?
Comment #9
mxh commentedThank you! I think this looks good. Will try to find some time testing it the next few days.
Comment #10
mxh commentedIt works!
Comment #13
ivnishComment #15
mxh commentedHm I see the PHPUnit test fails now. I can have a quick look.
Comment #16
ivnishThis is random test failing. I restarted pipeline