I agree that it makes sense for the contact form to be 404 if the email address is missing.
However, I disagree that providing no warning or user to the administrator of this fact is a good choice. It's right there on the same form; we can provide some sort of warning about it.
Not sure what the bot is on about but this is like a two-character diff in a file that's almost never updated; it will apply easily even if the source branch is out of date. Restoring RTBC.
Saving credits according to our core issue credit guidelines → . Thanks!
(Remember, closed issues get credits now, so any time you mark something wontfix/duplicate/etc., you're saving the credit records as they stood if the issue is from back when default credit was given for file attachments.)
So this was caused by the fact that the user was created by admin without an email address. So of course the user's contact form wouldn't work, and I suppose a 404 is a logical choice in that circumstance. But it says on their profile that their contact form is enabled, which is confusing. It would make more sense to tell them that their contact form won't work because they don't have an email address set (and to tell the admin this when creating them without an email address).
The contact tab being displayed to the admin on the user edit form when the email address is not set, only to show a 404, is also a buggy behavior. For an admin, it should just tell them the user doesn't have an email.
Oh, and just for completeness' sake: I granted anonymous access to these also remember, as per IS. Anonymous can see user 1's contact form, but gets a 404 for user 2's and user 3's.
I tried granting the user 2 admin permissions, and it could still see admin's contact form.
Except then I created a third user, and user 2 got a 404 for user 3's contact form.
I cleared browser cache, tried a different browser, etc.
Was going to assign it to myself to explain later but then ended up explaining it now.
The code is still essentially the same, so I should probably try to determine what I was talking about in my review of [#7173848-187]. The problem that I believe I was talking about was the following codepath:
if (isset($grant['langcode'])) {
$grant_languages = array($grant['langcode'] => language_load($grant['langcode']));
}
...in the scenario where the Language module or the language in question was not available.
language_load() not defined ➡️ fatal. This potential problem goes away with post-release Drupal 8 now that the language manager is always available.
Language not currently available on site ➡️ an array [$langcode => NULL]. Then the foreach would set it to a null langcode. That's the part I was asking for test coverage for, because this could potentially cause strange things for site owner expectations if a language is disabled and then re-enabled, for exampled.
But, crediting us.
They're still there and haven't been clarified. :)
Adding to saved credits.
FWIW, third-party settings does also sound like a better approach to me.
It would be fine in contrib... except that the fact that Views implements all sorts of code to do this. There's a Views refactoring issue postponed on this. That's why I didn't wontfix this in favor of it being a contrib feature long ago. :)
Still relevant AFAIK, and still postponed on the same core issue.
Since my suggestion would have been "Let's ask an Olivero maintainer" but @andy-blum already commented with equal uncertainty, wontfix seems fine. Thanks!
Saving credits for what began with a Hard Problems discussion that was one of the first steps toward the Autoupdates Initiative.
The contact form administration page also has a "Manage permissions" operation, which gives "No permissions found" for any form I create. Not sure if this is an extension point or what.
I tried adding a field to the user contact form (it shows up as having none; I guess the others are base fields rather than configurable?) but that did not have any effect on the bug.
xjm → created an issue.
Turns out, this is actually still an issue. The current string is:
When listing forms, those with lighter (smaller) weights get listed before forms with heavier (larger) weights. Forms with equal weights are sorted alphabetically.
This is also a total one-off in ContactFormEditForm. A proper fix would be to remove the custom description, or standardize it with that of other weight widgets.
Marking postponed since Contact is contrib-bound in the Drupal 12 cycle.
For what it's worth, the premise of this issue is not always true: Sometimes we are distinguishing "Add new" from "Add existing".
That said, core action links and buttons have undergone a lot of UX review and overhaul since this issue was filed and the general pattern is "+ Add thingy", so outdated is a good status.
This is actually outdated as Taxonomy fields are not Entity Reference fields under the hood. Thanks!
It does appear to still be relevant:
Well, let's stick with "outdated" so that people don't go "what are they thinking?!".
I've never been so happy to see a terrible bug be closed without being fixed.
Saving a few more credits since a nontrivial amount of work went into this one.
This bug actually still exists in Drupal 11, and in fact it's even more confusing, because the validation error happens off the screen for me. STR:
- Install Standard.
- Navigate to
/admin/structure/types/manage/page/fields. - Click "Create a new field".
- Click "Reference".
- Type "Tags" in the label field.
- Select "Taxonomy term" as the field type.
- Click continue.
On my 13" laptop screen, the form jumps so that the part that's failing validation (the machine name generator) is actually off the page. If I didn't already know what the bug was, I'd have no idea what the heck was happening, why clicking "Continue" did nothing, because there is no visible element explaining why it's not working. I have to scroll back up to see the machine name field that I (as a contributor to Drupal 8) know is there and failing inline form validaiton. That's quite the UX bug.
I think those look great, thanks @nexusnovaz!
Retitling this issue to its actual scope, which was far less sweeping than the title indicated.
For what it's worth, the original issue was filed under somewhat mistaken premises. We do (and already did at the time) have American English with the Chicago Manual of Style as our content standard for Drupal.org, and we also apply it to core's default translation and code docs as a best practice.
So "can not" is basically always a second-language speaker or someone from the Commonwealth. ;)
Adding credit for @swentel as his contribution here was also creditable. The rest of us not so much. :)
While I'm at it, updating the saved credits to credit reviews, substantive patch contributions, etc. and remove credit for simple rerolls or CS fixes according to our present core issue credit guidelines → . Thanks!
@mxr576, thanks for closing the duplicate and making a note that the credits still needed to be transferred over. Putting the text in bold is definitely helpful as such messages are easily missed. I'd suggest also adding it as a task in the issue summary "remaining tasks" in the future.
Adding credits for creditable contributions from the duplicate as per #58, and adding a note to the IS that this has been done. Thanks!
The other issue was also classified as a bug and a contrib blocker, which seems correct to me, so marking this as a bug also. Thanks!
Go team!
I gave @effulgentsia and @hchonov credit on the original issue instead, since there were a lot of other contributions there that had not been credited (major issue triage by the subsystem maintainers, various reviews, etc.).
🐛 Move entity preparation from form controller to entity class: "last changed" timestamp not updated and "create new revision" setting not respected when updating entities via Entity API Postponed: needs info was marked as critical by subsystem and core maintainers during a major issue triage for REST, so let's go ahead and promote this issue accordingly.
Also saving credit for various reviewers and discussion participants.
Adding credits for the triage from #34.
Wim Leers, gabesullice, effulgentsia
Let's use the proper duplicate status for that. :)
Yes, this was an API addition and contributed project blocker for Drupal 7, but I have no idea what the equivalent would be for modern Drupal and it was probably long since solved by the Entity Field API and its much richer validation and access checking.
Not saving any contribution credits since all the work on this issue predates the entire issue crediting system.
FWIW, "view mode" is still problematic, but I agree that having this issue open wasn't helping us solve that.
Crediting Jen for her UX audit of these issues.
Saving credits according to our core issue credit guidelines → . Thanks!
Speaking as a Views maintainer, I think the option described would overload the UI. I think this would be more appropriately implemented in contrib. Thanks!
Saving credits according to our core issue credit guidelines → . Thanks!
Saving credits according to our core issue credit guidelines → . Thanks!
Saving credits according to our core issue credit guidelines → . Thanks!
Saving credits for creditable contributions to the issue.
Adding credit for some reviewers, mentors, and discussion targets that should probably have been credited.
Saving credits for substantive contributions to the discussion. Thanks!
The major con from my perspective is that the backport policy would mean that bugfixes for the production branch, to say nothing of the maintenance minor branch, would inevitably start taking longer and longer as we approached a new major release. Multiple merge requests would be required, and it would move us away from what I think our primary goal is: Having a well-maintained production branch that is fixed quickly and easily when needed.
I also think it would get to be a real headache for contributors. If we're requiring them to have all the merge requests reading and not just one before commit, maintaining 3, possibly 5 merge requests on an issue, all of them going stale and needing rebases and needing merge requests resolved... that would get quite exhausting. We've realized that even 10.x is not a safe cherry-pick less than a year from the release of Drupal 11, so it does have to be separate merge requests.
And, if I'm reading the issue right and the scope was reduced, does that mean this was reduced to the project name only, and not supporting the version number problem?
https://packagist.org/packages/drupal-composer/info-rewrite seems to support only Composer 1 and 2.
Removing circular issue references.
My clever retitle of one of the related issues is a little misleading about just what old technical debt this is, and that the issue has survived not only multiple deployment best practices but also multiple d.do packaging systems and even core version control systems. 🤫
A followup issue from the great git migration got marked outdated recently, which, fair enough. The remaining scope of it was that dev versions od modules deployed with git deploy and then composer did't work with update.php. This looked like the modern home of that although this seems to only about naming, rather htan version numbers, abd to be yet another duplicate in a chain?
Is there an infra issue for #38?
It might be a dupe of #2559313: Parse composer.json for the project name for update module → at this point, however.
Titling to describe the remaining scope of the problem. As far as I know this is still an issue these days.
Thanks @akalata! Saving credits for those who worked on this according to our issue credit guidelines, including our upstream colleagues at TYPO3.
Crediting discussion and triage.
Duplicate is probably a better classification.
Saving credits for those who worked on this one.
I think duplicate is probably a better classification. :) This was fixed, just in an even better way than we planned.
Saving credits for all the contributors to this effort, which partly informed the improved field UI we have today.
Thanks everyone!
Sorry, there is a different meta I am confusing this with about the Image to Media upgrade path. This one can probably be called fixed now, rather than outdated:
I'll work on credits
I think we're closing outdated these days when really we just want an issue summary update. :)
The one issue in the summary about field descriptions was just closed dupe of a newer one.
This remains usability technical debt that was descoped and never addressed when we put Media in core, unfortunately.
I wonder why it never occurred to us at the time that the real confusion was $form_state versus #states.
Once the SQL storage is under the full control of the entity system
😢
More accurate status.
(Also, confused, the last updated date has like always been the default. But anyway, contrib would be the place for features like this since we limit the options on these prominent forms for usability reasons.)
This form has seen massive improvements since the issue was filed, so "outdated" is correct here. Even if there are still improvements to be made, we would want them to come out of a usability study of the current form for best results.
The method still exists and it's still a bad name.
I'm pretty sure the scenario in the IS is not possible because the storage prevents you from changing the cardinality if the field has data in it, thankfully.
The API and WTF-i-ness aspect might still be relevant, but I'm not brave enough to step down through that callstack to find out.
Adjusting saved issue credits according to credit guidelines.
I guess whether or not the date format links should open a new window is debatable, because technically they could be on a multistep form. I would still lean toward removing them. We can probably have an accessibility maintainer give feedback on that once we've identified the full scope of what core does.
A novice could work on this. The next step on the issue is to search for all the uses of target="_blank" in core, and document a list of which category the fall under as I've outlined them in the issue summary. Pay careful attention to the instructions in the issue summary.
Tag cleanup. (We don't actually need a11y review right now, and I did the IS update.)
Saving credits.
This issue has not really been maintained since we released Drupal 10, and is mostly no longer relevant. As I said on the parent:
With changes to both our release cycle and PHP's allowing longer support for old versions (and therefore more time to upgrade), we're moving toward 🌱 [policy] Default to requiring the latest stable PHP release available when a new major version reaches beta Active as a standard practice so that we overlap PHP's support cycle as closely as possible rather than trying to predict how well HSPs will keep up.
!
Thanks everyone!
Given that Drupal 10 is long since released with the 8.1 requirement and now in its long-term support phase, I'm going to mark this fixed. With changes to both our release cycle and PHP's allowing longer support for old versions (and therefore more time to upgrade), we're moving toward 🌱 [policy] Default to requiring the latest stable PHP release available when a new major version reaches beta Active as a standard practice so that we overlap PHP's support cycle as closely as possible rather than trying to predict how well HSPs will keep up.
Thanks to everyone who helped provide the data on their hosting providers so we could make a good decision for Drupal 10!
Aw, poor Locale.
Given that there is now only one open child issue (the other was wontfixed) and that the scope was limited to a set of Drupal 7 default views descoped from the module before we added it to core, I'm going to go ahead and mark this meta fixed. The poor Locale view can march bravely onward by itself. 🥲