- Issue created by @jan kellermann
- 🇩🇪Germany jan kellermann
jan kellermann → changed the visibility of the branch 3498834-dont-use-cores to hidden.
- Merge request !10866Disable storing and prepopulation from comment, user register and message form. → (Open) created by jan kellermann
- 🇩🇪Germany jan kellermann
Change record added: https://www.drupal.org/node/3498836 →
- 🇳🇿New Zealand quietone
Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to the Core change policies → .
- 🇬🇧United Kingdom catch
This was 'fixed' in #749748: Contact, register and comment forms do not prefill with user info from browser → without discussion of whether it was a good idea in the first place.
The only form this could potentially be useful on is the comment forms but anonymous comments on Drupal sites are incredibly rare. On user registration and contact forms this is a bug even without the privacy consideration imoz why would you want to prefill the register form of all places?
Haven't got back to the original issue that added the attribute but assume it was some time before that.
Two things:
1. Do we also need to remove form.js from these forms (or whatever the modern equivalent is)?
2. I think the test coverage on the user login form should stay until the feature is removed.
- 🇩🇪Germany jan kellermann
There are other behaviors in the form.js file besides fillUserInfoFromBrowser.
I have not checked if the forms use other behaviors (e.g. formSingleSubmit), but would recommend to deprecate the attachment later, as contrib modules or custom code could use behaviors from this file.
- 🇺🇸United States smustgrave
Possible to get a test case showing how this is an issue?
- 🇩🇪Germany jan kellermann
Test added.
Default tests ran successfully.
Default Test-only failed as expected:
There was 1 failure:
1) Drupal\Tests\user\FunctionalJavascript\UserRegisterFormTest::testRegistrationFormStorage
Written not strictly necessary Drupal.visitor.name to localStorage without consent.
Failed asserting that false is true.Please review.
- 🇬🇧United Kingdom catch
The MR is doing what I was asking for in #10 already - removing the #attached where it's not necessary. To me this is fine, custom form alters that somehow need form.js should also be attaching it.
Minor nit on the assertion comment but test looks good otherwise.
Long-term: Deprecation notice and remove the entire non-privacy-compliant function.
Let's open a follow-up for this, and maybe another one to audit form.js overall, it looks like there is potentially quite a lot of unused code in there.