Just chiming in with some advice:
setting display: block on images only to fix the bottom "whitespace" issue is not the best way, images should be inline by default.
A simple non-intrusive fix for the whitespace at the bottom of images is to just set "vertical-align: top" or any other statement except 'baseline' (I usually go for top).
Hope this helps.
I've tested the patches, and it seems to work well here.
No more warning that there is no upgrade path for token_filter, and in my testing, all tokens we used were preserved when testing this in our product on multiple sites.
I can't review the code itself, but functionally it seems to work well!
For future testers, it was a bit confusing at first what patch applied where, if you want to test this patch you have to:
Apply patch #17 to the drupal/token module.
And the patch for drupal/token_filter i've attached to this comment: it's the same patch as the "MR5!" patch, but can be applied without worrying that the merge request will change in the future and perhaps breaks your site.
Thanks for all the work everyone!
Works!
Installed the module in a fresh drupal 10 site.
cloned the drupal-9 version of this module in the contrib map, and applied this patch.
after that I could enable the module without problems.
Created a node, and manually created an alias for the node, with the "fixed alias" enabled.
Drush cex exported the setting correctly!
removed the alias from the drupal site, and did a drush cim.
It restored the previously exported alias.
So everything seems to work correctly!
In this patch i've added a unique ID to the empty-result div.
That ID is referenced by the new "aria-describedby" on the search input field.
I've also updated the description of the field in the admin form.
It used to be:
"Enter the HTML to show when the search input gets focus and there is no search term in the input. Useful for "quick links" for instance."
I've changed it to:
"When the search input gets focus and there is no search term, this text will be shown and is read by screen readers. Keep this short and do not use HTML to avoid accessibility failures."
This field has been 'abused' a bit to show some quicklinks, but those links were never accessible to screen-readers (screen reader users didn't even know that text existed) and those links are not reachable by keyboard users.
So it's better to update the description for it's modern use.
Showing a list of quicklinks (when you haven't entered a search term yet) that's accessible should be built completely different.
This patch shouldn't 'break' any existing sites using it for a list of links, it just exposes the problem they already had.
bskibinski → created an issue.
Simple patch that changes the text to the one proposed in the ticket:
Navigate up/down to select results or confirm directly to view all results.
bskibinski → created an issue.
Because of a recent accessibility audit, I see this new functionality for extlink.
I would advise rolling this change back, but that could be difficult now without breaking existing sites.
The problem of rendering the icon/text outside of the link, is that screen readers (accessibility) won't read this text, because it's placed outside of the link, and isn't referenced by any aria-describedby labels.
IMHO I think a better solution would be to just override the underline on the icon with CSS, instead of this solution.
We do want to make an improvement, that decouples the icon from the 'screen-reader' text, and than this option would be viable again. But for now, I would not recommend using the new before/after placements.