🇺🇸United States @tedfordgif

Hmmm, another terrible idea: I wonder if you could implement the transition idea without JS using radio buttons. If previously checked, make the "checked" input have an empty value, and the "unchecked" input have a value attr that means "remove this permission from the role". You could even use CSS to get close to the current interface.

I'm guessing the radio value always gets submitted, though, even with a blank value.

And there are likely to be a11y concerns.

Aren't there "shadow" hidden inputs as well? In order to get rid of those you'd need to e.g. round-trip a hash of the current state to detect if another user saved permissions in the meantime. Then the front-end could submit only the checkbox transitions. That would require JS as well.

While we're talking JS, if we send the max_post_vars as a setting, we can stop the submission if we would exceed it.

Or turn it into a progressive enhancement exercise, where the no-JS form can only look at one role at a time, and only once you enable JS can you edit all roles at once.

Another approach might be to simply diverge from core's handling of URLs, and make redirect_404 everything-sensitive, e.g. throw 'binary' on the 'path' field in the redirect_404 schema. That would be unexpected for users/devs, and is probably not correct, but would have the same effect as the URL-encoding suggestion (absent normalization).

This is not the right ticket to discuss it, but another place we end up with inconsistencies is that public files don't get served by Drupal, but the 404 page for the same does. Should core have a redirect if the file_managed uri matches the path modulo collation? That would also mean fewer redirect_404 records would get created.

The issue with allowing the database collation to determine matches is that we may want different behavior in different circumstances. For example, if a single character in /some/long-URL-slüg has a diacritic mark like that, we probably want to serve the same page, especially if the canonical path has the diacritic and the request path does not. However, in a short url slug, such as you might see on /a/view-page-with/tags/slüg, some languages probably have word collisions we wouldn't want. Maybe the correct approach is to always have unambiguous short slugs (add context to the "machine name", e.g. garden_slug).

Adding core issue for reference on handling case sensitivity, but with only one mention of "collation".

Thanks for organizing, Laura!

Thanks for organizing, Laura!

laura.gates → credited tedfordgif → .

Based on last comment, the issue was resolved and can be closed.

It definitely looks like an error would have been thrown and logged: SplFileObject->fgets() calls spl_filesystem_file_read_ex() with silent == false, which will throw a PHP exception if the file can't be read.

That's the only place in the PHP source I see "Cannot read from file", so if we found that in logs it would be pretty clear.

As to how that can happen in ExtensionDiscovery::scanDirectory(), I'd look for race conditions if we're running tests in parallel, or some other filesystem failure (network filesystem, running out of disk space, ...).

@alexpott the assumption would seem to be that having a reference to an open SplFileObject (with a recent !eof check, no less) means you can read it without errors. That seems like a reasonable assumption to me, and matches most of the usage examples I've seen. However, perhaps we should consider adding a call to SplFileInfo->isReadable() before calling openFile(), and maybe logging our own error if not. There may be some minor performance implication if isReadable() and openFile() don't share a cache.

I also find the differences between SplFileObject::eof() and ::valid interesting, although the latter is billed as !eof.

This is fantastic work.

Adding to #3317769-43: Drastically improve Drupal's default linking experience in text fields → , the ability to select within the link dialog which type of link you want (e.g. for a given media item, do you want to link to media detail page vs file download) could be done in a follow-up issue. However, it might make sense to have some architectural review now to make sure it is possible in the future. For most of my sites a single link type will suffice, but there are occasional sites with e.g. a public media asset catalog where it would be nice to have that flexibility.

Options include:

• \Drupal\Core\Entity\EntityLinkTargetInterface::getLinkTargets() returns multiple links
• Allow multiple handlers in the link_target annotation, just like link templates
• Maybe integrate with the link templates themselves somehow, e.g. maybe I want a link to edit the given entity

Added draft change record https://www.drupal.org/node/3350667 →

I'll work on the CR.

I like the idea of having both the explicit x-drupal-maintenance-[something] and the retry-after header, the latter having a random range and no-UI min/max settings like for JSON:API. However, perhaps for BC reasons retry-after should not be added to the response until a dev adds the settings (or makes them > 0)? The behavior is then different than JSON:API, but that could be harmonized in D11.

Spurious test failure disappeared after re-running the tests, back to NR.

My apologies for leaving out the assignment to $build, I'll edit my comment.

Given that the setLimit() method returns self, it probably makes sense to test that contract in the test, e.g. instead of these two lines:

+    $list_builder->setLimit(FALSE);
+    $build = $list_builder->render();

just this one

+    $list_builder->setLimit(FALSE)->render();

Although Solr is the primary backend we use, I find myself considering the db backend for specific use cases when Solr is not available, sometimes on large sites. I like the option of not touching existing indexes, and enabling it by default for new indexes with a sufficiently noticeable warning. Thanks for your work on this!

I'm not sure there is a general solution here, but this probably belongs on the Search API project. For example, Elasticsearch supports null value replacement when indexing, notably with the same-type restriction mkalkbrenner discussed on Slack (can't index a "NULL" string for a long int field).

An alternative general approach would be to create an additional field for each nullable field in the index. E.g. for solr create is_field_number and null_field_number, the latter being a type="boolean" field added to the schema.xml. Search API would add the general support, and each backend would declare support and implement it differently depending on the type of the field. I'm not sure this actually works in practice, and it is complex.

Perhaps instead of the general/framework component, this issue should be scoped for only the Views integration? Is there another place where empty values cause extra entity loads? Maybe just spin out a separate ticket for that approach.

As the original reporter of the issue, I think it's time to close this. Frankly the title contradicts itself, a root-relative URL isn't a URL. As Berdir pointed out, there have been related issues reported, some merged, which I'm now linking to this issue. Although we could convert this to a simple documentation update (my proposed alternate approach, patch attached for posterity), one might argue that the buildUrl() docs have always linked to the relevant methods for converting to root-relative links, so even this documentation patch is not really needed. Thanks to everyone for their contributions, but let's help clean up the issue queue by closing this.

If the MR is against 10.1, might as well update the issue metadata.

Updated tests to include all elements with the datetime attribute. This doesn't give us any extra code coverage, but perhaps it helps prevent a regression in the future. There's an alternate patch if a single test scenario is preferred to three.

Thanks for your work on this, @smustgrave. In your screenshot, the datetime attribute is the issue, not the #DBD text

1978-11-19T05:00:00Z is having the longest potential "protocol" left-trimmed, since Xss::filterAdmin treats any text followed by a colon as a protocol. That results in datetime="00Z", which is no longer a valid datetime attribute.

@neclimdul, thanks for cleaning up the patch.