Account created on 18 February 2011, over 13 years ago
#

Merge Requests

Recent comments

🇺🇸United States itmaybejj

Oh one last note -- ideally, if you are able to add JS to the site, you can get rid of these alerts altogether by automatically opening elements to reveal the hidden content with the error before Editoria11y runs the hidden check.

🇺🇸United States itmaybejj

Oh I like this idea.

I need to rewrite that whole part of the code as part of the CKEditor integration rewrite, so I'll add this to the queue for that.

🇺🇸United States itmaybejj

Looking this source code over (have not pulled a test environment) --

I would not drop aria-describedby and aria-labelledby if the targets are rendered on the page. I see your logic, but if those attributes are present for some reason, won't they be equally needed in the duplicate?

Also -- I'm assuming if there is no aria-label on the source, this code won't put that attribute on the target, as opposed to putting an empty aria-label? I've seen the latter sneak through now and then.

And last -- aria-details does not have widespread support. It doesn't hurt to copy the attribute, but developers really should not be using this attribute in production code.

🇺🇸United States itmaybejj

Oh I thought that happened automatically through commit credits

🇺🇸United States itmaybejj

I'm closing this for now. The work it would take to add variables for all the strings to the GUI and map all those variables to the localization file is just out of scope for me. If someone wrote it, I'd review it.

At the JS level, the localization file can be overwritten with your own values. Or you can use the ed11yPop event to modify the rendered tips however you want -- e.g., by adding that help link.

That said -- the default language in the tips are just the result of this community winging it. I heartily welcome suggestions for improvement.

🇺🇸United States itmaybejj

I'm closing this for now as it is working as designed; please re-open if I have misinterpreted.

🇺🇸United States itmaybejj

...fwiw my prolonged silence is me wondering if I can be a little more lightweight here. This many cache tags down on a cache tag that is only set for logged-in content editors and only invalidated if someone dismisses a manual check (...rare)...it wouldn't be a big deal of a couple extra pages got matched for invalidation...so rather than using an MD5+b64 conversion, I could probably just change the string truncation from the first 1,000 characters to something like the last 20.

Thoughts?

🇺🇸United States itmaybejj

I don't know how to hit the strings in config/YML, but hopefully this commit will help with the user-facing interface.

🇺🇸United States itmaybejj

Out of curiosity if you know -- is the automatic word matching layer aware of context cues, or are they purely for the human translators?

note to self for implementation -- context syntax

🇺🇸United States itmaybejj

What is the markup being output? If all is working correctly:

  • no alt attribute: error, missing alt. Screen reader behavior varies, but generally speaks filename or file url.
  • alt="" (empty quotes): warning/manual check for decorative image. Screen reader ignores image.
  • alt="""" (quotes containing quotes): error, because this is not marked decorative, but rather is technically valid alt text made of quote characters. Screen reader announces that there is an image, but just makes an awkward silence when it reads the alt as it iterates through characters it does not pronounce.
🇺🇸United States itmaybejj

Oof. So this module family creates a pseudo-multisite with multiple domains being served from one DB?

If that is the case, I don't think we should drop the domains from the url -- you'll want them later, to search and filter the API reports. Probably better to rewrite the not-much-better-than-a-placeholder Editoria11y API path validator to accept absolute paths. Maybe that should be replaced with a call to PathValidator or something like that?

This may have some downstream affects on the views as well; that would need to be tested once the API was accepting absolute URLs.

🇺🇸United States itmaybejj

I haven't been able to reproduce this yet -- is it missing all <picture><img> elements on your site, or only ones in certain containers?

By default that particular test ignores the following containers in addition to any custom settings:
img[aria-hidden], [aria-hidden] img, .ckeditor img

Is there any chance the missed-missing alt matches one of those?

🇺🇸United States itmaybejj

Yeah I don't think I want to support this directly because it makes me nervous about the future of the data. E.g., uninstall the module and now you have images all over the site with a meaningless string in the alt field being printed to the frontend. Better to just write a good alt the first time, and treat a missing alt as incomplete content. Really very few images should be marked decorative.

That said-----you can select directly against the alt attribute itself in the ignore selectors: [alt="technical debt"]

🇺🇸United States itmaybejj

Hi Bernard,

Have you played with Decorative Image Widget ? That might fit your needs for the content editing side.

There is not a connection between the two, though -- the widget lets the author mark the image as decorative, but Editoria11y still asks them to confirm that the decision is OK once the image is in context on the page.

I do not have an exposed setting to remove that second check, but one could disable that second check by setting an ignore selector like img[alt=""].

🇺🇸United States itmaybejj

I found additional problems that block disabled users, stemming from the same cause.

  1. Screen readers are not reliably detected. Screen reader users can navigate without using the Tab key, and several navigation methods throw no JS events at all. So only meta keys may be received, or it's possible NOTHING will be received before other than a click on the "next page" button.
  2. Voice control (dictation) users are not detected at all. The first event their browser fires is "click" on the submit button.

I pulled your patch into an issue fork and added some more:

  1. Simple click detection to permit users relying on assistive technologies to submit the form. It's the only event that some browsers are going to fire I'm afraid.
  2. Config settings to make the error messages customizable, so that real human users who are blocked for any reason can be given a fallback contact method, such as email or phone.
🇺🇸United States itmaybejj

fix broken edudu link

🇺🇸United States itmaybejj

We discussed offline; it looks like the issue isn't coming from the Editoria11y codebase.

🇺🇸United States itmaybejj

Oh and the other thing that would really, really help me would be seeing if there are error log messages at /admin/reports/dblog right after enabling the module. The one time I've seen something like this in the past was when a very particular PHP/SQL combination didn't like something in the table definition and refused to create the table.

🇺🇸United States itmaybejj

That's...really quite troubling. I'd...assume at first glance that the module install hooks failed altogether, or that update hooks haven't run.

Could you tell me a LOT more about how I might set up an environment to reproduce this? Fresh install or upgrade, anything you can tell me about your host, and everything in "General System Information" under
/admin/reports/status

Feel free to email me if you don't want that public (jjameson at princeton.edu).

🇺🇸United States itmaybejj

I think this will fix the issue -- want to test before I tag a release?

I added a new, separate parameter, with a default value that should work when blank.

🇺🇸United States itmaybejj

This should take care of it; do you want to test before I tag a release?

🇺🇸United States itmaybejj

ok as I suspected. I'll see if I can revert that for a bit.

🇺🇸United States itmaybejj

Hmm good find. Looks like I hard-coded the default strings from the ExtLink module rather than exposing a linkNewWindow parameter.

Lemme fix that.

🇺🇸United States itmaybejj

oops sorry didn't mean to bump the status

🇺🇸United States itmaybejj

This is huge; thank you thank you thank you

🇺🇸United States itmaybejj

Here's my guess -- I'm studying that particular test, and as written it relies on a feature that was added to Firefox in version 119. The Firefox ESR channel is still pinned at 115, until sometime this summer.

I'm...hoping you can tell me that the affected Firefox version is 115.x.

If that's the case, we just need to decide whether to document this as a problem that will fix itself soon, advise y'all to add a[aria-label] to your ignore list, or have me remove the shorthand for the old verbose version.

If not...the puzzle continues, but I suspect the answer might be the same.

🇺🇸United States itmaybejj

Thanks; that's enough information for me to be able to reproduce the issue. Lemme see if I can puzzle this out.

🇺🇸United States itmaybejj

It's true, but this is a regression that wiped out some seriously key stuff for onboarding new community members.

I guess if nobody else thinks this is worth fixing I'll just go into all the Drupal doc pages I can find that tell developers to read the render API pages, and change the links to point to the last-good d9 URL. That will create some terrible technical debt, but I can leave a list here so they can be changed back if this fixed.

🇺🇸United States itmaybejj

On the accessibility question -- I don't anticipate an issue there. Assistive devices only need an HTML ID if something in the HTML is trying to reference the element by its ID, e.g. via a label's for= attribute or an aria-labelledby reference.

Sometimes in nav elements it's nice to reference the inner heading to give the nav a unique name. But that could be done in the JS with a simple global counter.

🇺🇸United States itmaybejj

It's worth noting, if you are triaging docs, that a decent percent of the existing docs are broken 🐛 Classes missing from Drupal 10 Fixed .

I think it would help, regardless of what system we use, if there was a core documentation template focused on the person who is going to use but not understand the code. Write it for the new user. A lot of the existing function/class documentation is along the lines of "X: provides an interface to make an X."

E.g.,

  1. Purpose of function
  2. Example use
  3. Links to extended how-tos

If that was templated, it might also help keep track of out-of-date documentation, since somebody doing a code review would see the URL that would need to be updated with the change.

🇺🇸United States itmaybejj

We'll have to find another issue to keep you busy after DrupalCon!

all done

🇺🇸United States itmaybejj

Not being able to comment is less broken than not being able to read the documentation...

🇺🇸United States itmaybejj

I also got here trying to find the list of render elements, and begging for help on Slack.

If this is going to stay broken, can we set the default branch at api.drupal.org back to 9.x until it's fixed?

Having the core dev documentation broken for the project is not a great look, and it looks like Google has dropped the results for all the API render elements.

🇺🇸United States itmaybejj

Yeah that would be an easy feature to add, since I already baked the parameter into the library.

I've got a busy week ahead so I can't promise a quick turnaround.

🇺🇸United States itmaybejj

Yes! Sorry I was staring at JavaScript all morning working out this bug and some others I noticed while testing the fix. Just got it working and pushed 1.0.5 .

And yes definitely follow up. We rolled it out to our ~1,000 sites this week, so I expect most remaining bugs will be just like this -- config combinations I didn't test well enough.

🇺🇸United States itmaybejj

Thank you! I didn't know about this service.

Merged & released.

🇺🇸United States itmaybejj

Hi folks -- just wanted to revisit this, because the parameters have changed. Please check the 2.1.11 release notes for details.

🇺🇸United States itmaybejj

Alrighty then. That last commit abstracts all the CSS back out into a remote file, which removes the need for any CSP hashes or nonces at all.

This was how I did things in the 1.x branch. It makes development much more complicated for this sort of module; it makes theme conflicts more likely, I have to inject duplicate references to the file into each shadow component, and I can no longer compute values based on params that can't be converted to CSS variables...but...it looks like it works. I'll need to test more and update the docs before tagging a release...hopefully within the week.

Production build 0.71.5 2024