Account created on 18 February 2011, about 14 years ago
#

Merge Requests

Recent comments

🇺🇸United States itmaybejj

Hmm....unless there is a bug, if "Show tips automatically when issues are detected" is set to anything other than "Assertive" (the default), it will only auto-draw the tips if there are new errors on the page. Themers repeatedly refreshing a page should only have stuff tossed all over their work once if they close the toggle and refresh. Subsequent loads will read their local preference for "shut," see that the number of issues hasn't changed, and start minimized (e.g., as an on-demand check). Maybe try setting that to "On first detection" or "Polite" during development and see if that satisfies your themers? "On first detection" used to be the default, but too many editors were ignoring the toggle...

I would consider a merge request to add a per-user preference for "Show tips automatically when issues are detected."

🇺🇸United States itmaybejj

I...don't plan to contribute my own time to this -- if it's modifying inline DOM (other than the toggle), there's presumably a problem on the page. And if that is breaking a component, the themer needs to account for it, since it will break it for front-end editors. And if the toggle is in their way, it's going to be in the editors' way too. So I don't let our themers turn it off; I want them to be putting as much thought into the editor UI as the frontend.

As of the latest release they can hide all duplicates of a manual check on a page for themselves. They could also split out their account to a role that does not have the checker enabled. Or they could temporarily add .ed11y-element {display: none;} to the CSS...assuming they don't accidentally commit the change...

Or they could throw this in dev JS to let them modify anything for themself at runtime, ideally wrapped in an if for drupalSettings.user.uid:

// Tell Editoria11y to call our option override.
var editoria11yOptionsOverride = true;

var editoria11yOptions = function (parameterBag) {
    let options = parameterBag;

    // Dev approach 1: do not draw interface at all.
    // This overrides the GUI config for "Disable the scanner if these elements are detected."
    options['preventCheckingIfPresent'] = '.some-devel-button-that-only-themers-see';
     
   // Dev approach 2: ignore config and force polite mode.
   options['alertMode'] = 'polite'; // Override any 'assertive' config.
   options['userPrefersShut'] = true; // Override localStorage for open/closed state.

   // Dev approach 3: only run on click:
   options['ignoreAllIfPresent'] = '.some-devel-button-that-only-themers-see';
   // That's a library param that isn't in the GUI, the opposite of the GUI one:
   // "Hide all alerts if none of these elements are present."

  return options;
}

If they are the only themer on the project, they could also just add ".some-devel-button-only-themers-see" to the GUI config for "Disable the scanner if these elements are detected."

Mostly...I just philosophically do not think putting something in the GUI that lets end-users to turn the checker on and off is a good idea; this is one of the key differentiators between Sa11y and Editoria11y. Sa11y allows more local-user control, Editoria11y is centrally managed. I found that users who could turn things off, did. And then they forgot to turn things back on.

I would consider a merge request if it let the admins control who was able to do this.

🇺🇸United States itmaybejj

Oh goodness that should have been caught in testing; the title string never got written for that field.

Those boxes are for tests to turn off. I'll fix that in the next release.

I don't plan to add an include/exclude toggle, just because tests come and go and various sites have custom tests, so a simple "ignore" param is much easier to manage in the library. I'd consider a clever merge request if one came through.

🇺🇸United States itmaybejj

That's a good idea. I'll track my work in #3521111.

timwood -- I'd recommend making the same feature request in back-to-top. Because that displays late I've seen it have issues with other modules. They are welcome to yoink my code.

🇺🇸United States itmaybejj

Sure I can add this in the next release. I already added the param to the library recently, so I just need to expose it on the config page.

🇺🇸United States itmaybejj

It already does! just swap the filenames or contents of editoria11y.libraries.dev.yml and editoria11y.libraries.yml during development -- the dev.yml library definitions are for the uncompressed/unpackaged files, which can be found in /library/js/

🇺🇸United States itmaybejj

Fair enough. The functionality is by design, but maybe I should revisit the design in a future iteration so there is a visual distinction between the editable headings and the template headings. Though in a lot of contexts I just won't know which it is; there are often a lot of template headings within the editable area.

🇺🇸United States itmaybejj

Is it flagging things as errors outside the specified check areas, or just showing other headings in the little visualizer?

What I am trying to do at the moment is have the page outline visualizer ignore the check roots and show all headings, but for errors in headings to only be flagged if they are within the specified check roots. But that means I'm running the same DOM dive two different ways and have to keep track of which tree I'm using at all times, so errors on my own part would not be surprising...

🇺🇸United States itmaybejj

Indeed.

I think the best thing here because of the late fade-in is to use CSS to move the back to top widget instead, so you don't get a weird floatie-thing going where Editoria11y is sorta vaguely centered before the other widget appears. Something like:

body:has(ed11y-element-panel) #backtotop {
  margin-right: 140px;
}

I'm a little loathe to add that sort of thing in my module, since I'd be moving their widget; might be good to ask Back to Top to add an offset parameter?

🇺🇸United States itmaybejj

Ahh yeah; I'm afraid that explains it. Injecting my code into the CK4 iframe was not feasible.

I'll convert this to a documentation task then, so I don't forget to make this clearer in the next release.

🇺🇸United States itmaybejj

It should check within CKeditor5 on a node edit form, but it should not appear on the admin theme unless a CKeditor5 instance is present and the Layout Builder editor is not currently upen. It is not compatible with CKeditor4 or the Layout Builder editor.

The module readme appears to be out of date. Checking while editing rolled out right before DrupalCon.

If you are not seeing it on a node edit form with a CKeditor5 instance on screen, using stock configuration, I'll have some followup questions as there may be some new module conflict I'm not aware of.

🇺🇸United States itmaybejj

Link indication contrast in body content

Got a WCAG issue that shouldn't be too hard to solve -- the links in body content (module info pages, these comments, etc) are hard to see because they only have a 2.8:1 contrast ratio against their surrounding text. 3.0:1 is the minimum a lot of people can see:

This in theory could be fixed on a white background by using a lighter blue, but that's just not going to work because the same link color is used on gray backgrounds (e.g., the "Issue Metadata" box), and a lighter blue will then fail contrast on the background.

So the typical solution is to make the links stand out using a secondary indication -- e.g., underline:

#main a[href]:not(.button) {
  text-decoration: underline;
}
#main a[href]:not(.button):hover, #main a[href]:not(.button):focus-visible {
  text-decoration-style: double;
}

Or bold:

#main a[href]:not(.button) {
  font-weight: bold;
}
🇺🇸United States itmaybejj

Yup it looks like your theme is inserting a gap between elements, so adding elements adds gaps:

.menu .primary-nav__menu-link > span {
  gap: 0.5rem;
}

This would fix it:

.menu .primary-nav__menu-link.link-purpose  > span {
  gap: 0;
}
🇺🇸United States itmaybejj

So; that extra markup is most of what this library does, rather than CSS ::after pseudocontent. The span wrapper prevents the link from breaking its line between the last word and the icon. But yes, sometimes themes need to style the icons to make the spacing match the text in menus, especially the icon width and height. This is the CSS provided by the library.

If you provide me a link I can take a look.

🇺🇸United States itmaybejj

I'm flagging this as fixed for now because of testing on a copy of your DOM. Please test 2.2.5 with the new param "Auto-detect any Web components" turned on under "Web components and custom tests," and reopen if I haven't succeeded.

I have that param off by default for now until I'm confident it both works and doesn't hurt performance. This part of the code usually finishes in less than 1ms so I don't think it's going to hurt performance in any noticeable way.

🇺🇸United States itmaybejj

Switching this to Fixed because I think 2.2.5 will prevent future folks having the same headaches.

🇺🇸United States itmaybejj

This looks fantastic to me. Thank you. Merging to test for next release...

🇺🇸United States itmaybejj

Hello Nitesh,

I do not know how to provide a schema for a View. Could you assist?

🇺🇸United States itmaybejj

If y'all are up for testing something, I'm hoping MR 25 will take care of this.

🇺🇸United States itmaybejj

oh that's GREAT to hear! I'll keep focusing on general performance improvements then and not worry so much about testing against this site in particular.

🇺🇸United States itmaybejj

After some more thought and testing, I think what I'm going to do is add some config for whether Editoria11y should watch the page for changes, and wrap most of the new code into that. That should give 2.2 a static running mode that performs like 2.1.

🇺🇸United States itmaybejj

I should have a release ready for this in April.

🇺🇸United States itmaybejj

Meanwhile -- see if that change works for you? It moves the message from the Help hook to the post_update hook, which only fires after the module is installed and the route exists. I hope...

🇺🇸United States itmaybejj

Questions --

  1. There's no route named editoria.settings in the module repo -- is that a typo here in the queue, or is it possible there's a patch/typo in your dev environment?
  2. It looks like this fix requires the Help module to be both installed and configured? I'd rather not add that dependency if possible.
  3. I'm having trouble reproducing this. Is the install failing on your end, or is the install finishing and some local test suite is running before a cache clear and throwing errors in the console?
🇺🇸United States itmaybejj

OK fair enough.

For what it's worth I asked the other maintainers to weigh in, and was reminded that they don't like that I automatically set permissions in the first place. Ahh well; it's the endless argument between "turnkey for most sitebuilders" vs "minimum viable install for developers." I'm biased to the former.

Glad you're enjoying it; it's a monster of a project!

🇺🇸United States itmaybejj

I am afraid you are going to have to convince me it is a good idea to give a non-author role an elevated permission to view unpublished content they have authored. If they cannot author content, then there are no nodes for this permission to give them access to, which is why it is not enabled by default for non-editor roles.

So as far as I can tell the only non-authors this permission would make sense to give to would be people who were previously authors and had their author role removed, but you still wanted them to be able to view (but not edit or publish!) something they created but did not publish before they did whatever they did to lose access to the ability to create new content or publish that content. That seems...awfully unlikely, and far from least privilege best practices. Unless there are different use cases in play here I'm not thinking of -- e.g. some form of form-based content creation that non-authors can use where they need this permission?

...so that's why I have been using this permission so far as a proxy for "can edit stuff." It seems to be the only universal "can edit stuff" permission on the default list. The others aren't always enabled on all sites (can use toolbar, can use admin theme) or depend on particular content types being present...

...but note that it just adds the permission on first install. You can drop it on your sites if you want to keep your roles configuration as-is.

🇺🇸United States itmaybejj

It only auto-adds permissions on install for roles that already have one of the following permissions:

if (
      $role->hasPermission('view own unpublished content') ||
      $role->hasPermission('access content overview') || 
      $role->hasPermission('access in place editing')) {
                $role->grantPermission('view editoria11y checker');
                $role->grantPermission('mark as hidden in editoria11y');
     

This is my attempt to detect editors, not just authenticated users, so that the module "just works" for most sites immediately on install. Site admins can then remove the permission if they do not want it on a particular role.

Do you have one of those permissions assigned to non-editors?

🇺🇸United States itmaybejj

Since I need to rewrite the backend anyway to take part in the dashboard initiative, I think I'll work on that first -- once all the data in the dashboard can be fully parsed by views, I can replace those links with a dependency on Views Data Export , which has all that code already written.

In the interim the easiest solution would probably be getting a count of the rows, and then exposing a filter to download some reasonable number per report.

But I don't think I'm going to work on either solution myself until I finish the backend rewrite, so I'm going to mark this postponed unless someone wants to contribute a merge request.

🇺🇸United States itmaybejj

Interesting. This might work.

I'm on assigned projects pretty much to the end of the month, and will be turning back to this March-ish.

🇺🇸United States itmaybejj

Oh that's a great idea. I should add that too.

🇺🇸United States itmaybejj

Editoria11y also flags alts that are suspiciously short, suspiciously long, contain common placeholder text for validation evasion or trim() to 0 length if you remove unpronounceable characters like "?,.'. The first two are "soft" alerts, the latter three cannot be dismissed.

If you are validating server-side, you could potentially OCR the image as well and flag images of text. That's too computationally heavy for me to do at runtime.

🇺🇸United States itmaybejj

The menu seems possible. And it is outside main, so hopefully Editoria11y's mutation observers are ignoring all those changes.

The one clear takeaway I see so far is that I should make Editoria11y inherit the query token from Drupal for the CSS file rather than using its own version number; that should guarantee that the CSS preloads with the document before initial paint, so there is no second network call.

🇺🇸United States itmaybejj

Oh that's clever, and will work fine for the life of the 2.x branch. Sa11y does something similar, just embedded within the rulesets.

My vague plan has been to use an array like that, but reversed (disallowedTests), so that people would not need to update their settings when I added new tests and there wouldn't need to be as much data in drupalSettings.

🇺🇸United States itmaybejj

Great!

Yeah I'll leave this open so I remember to include this in the next release.

🇺🇸United States itmaybejj

Oh dear; looking at this...I think I need to update that documentation, because the library is aggregating the minimized JS file now, so doing it that way is going to get more complicated...

If you only need to override one, it would probably be easiest to just to override it at runtime; this could go in any theme JS.

// Listen for event

const overrideEd11y = function() {
  Ed11y.M.linkNewWindow = {
      title: 'Manual check: is opening a new window expected?',
      tip: () =>
        `<p>Readers can always choose to open a link a new window. When a link forces open a new window, it can be confusing and annoying, especially for assistive device users who may wonder why their browser's "back" button is suddenly disabled.</p>
                <p>There are two general exceptions:</p>
                <ul>
                    <li>When the user is filling out a form, and opening a link in the same window would cause them to lose their work.</li>
                    <li>When the user is clearly warned a link will open a new window.</li>
                </ul>
                <p><strong>To fix:</strong> set this link back its default target, or add a screen-reader accessible warning (text or an icon with alt text).</p>
                `,
    },
  document.removeEventListener('ed11yResults', overrideEd11y);
}

document.addEventListener('ed11yResults', overrideEd11y);

🇺🇸United States itmaybejj

I'm not working on anything tagged "postponed" at the moment.

🇺🇸United States itmaybejj

Oh yes -- don't worry about selectors for the admin theme. If Editoria11y detects editable content it swaps out that parameter for its hard-coded selectors for CKEditor and Gutenberg instances...and it doesn't render on admin pages if it doesn't detect editable content.

🇺🇸United States itmaybejj

Oh and -- config: content_root: '#main, .block-system-main-block' -- I'm looking at your site and those selectors are nested. Editoria11y does not like content roots being nested within each other. Can you knock one of those out?

🇺🇸United States itmaybejj

Oh that makes me want to cry. I used to inline all my CSS until the CSP module folks Support Content Security Policy (csp) Needs review asked me to abstract it out, and remote CSS calls are thread locking. But still; those late CSS requests should be served from cache, so only the first should slow you down, which makes me wonder if something is running for 55 seconds before the locking request is made. What I really want to see is the JS stack chart for those 55 seconds.

eeeeven more diagnostic questions:
Do you see the same problems while editing pages (backend theme) or only frontend pages?
On the frontend when you are seeing issues, what is the value of Ed11y.options.inlineAlerts?

🇺🇸United States itmaybejj

Oh that's very helpful. We all run LastPass too. I'll see if I can figure out how to record and trigger that.

🇺🇸United States itmaybejj

Yeah that might do it. I'm on project work for the next week but I'll give it a spin when I come up for air.

🇺🇸United States itmaybejj

Hmmph. That config should be fine.

I was able to grind my browser to a halt on one page on the frontend. I don't know if it's the same cause, but there it seemed that Editoria11y and Qualtrics were fighting with each other -- page interactions were causing Qualtrics to tie up the main thread, right when Editoria11y was trying to run. And since Editoria11y was asking for animationFrames to try to move its tips to sync with page scrolls, this was leading to more than one second of jank:

Are you perchance running Qualtrics metrics on the backend as well as the frontend? If so, do the problems go away if you set it to only render for anonymous users? Editoria11y 2.1 did not have the mutation observers to check for changes to the page, so it would not have been trying to run at the same moment during page interactions.

I can remove the animationFrame requests and see if it improves anything, but since JS is singlethreaded I doubt that will make a difference; any activity ties up the thread. I think something bizarre is going on, like a memory leak of some sort. I'm very puzzled, because my browser would still be lagging heavily even though the performance monitor was not showing any JS activity.

I have to set this aside for the moment because I am not sure what to do next, but I am going to keep this bug open until we figure it out.

🇺🇸United States itmaybejj

Which site is this for? I'd like to look at the markup. Also, could you send me a copy of your config settings? I have seen some situations where certain settings can add up to a lot of drag.

I performance test on synthetic pages with hundreds of issues and a literal copy of Moby Dick, so if you are having any performance issues it is a bug, and I would like to troubleshoot. 2.2.x should not be slower than 2.1.x. The one thing I can think of that has changed are some mutation observers -- if you have mouse hover based attribute changes in the DOM it is possible something is thrashing....

🇺🇸United States itmaybejj

Oh my goodness; this is the first time I've seen a nested Web component. I think both my element finder and name computation only recurse once and will need to be rewritten. I'll reopen this issue.

🇺🇸United States itmaybejj

Sure, if you're thinking more along the lines of input validation than maintenance down the line.

🇺🇸United States itmaybejj

Marking fixed for now. Please reopen if you notice issues.

🇺🇸United States itmaybejj

Although if I took that route -- I have often thought about creating a browser-based simple crawler for Editoria11y, that just creates a grid of iframes and runs through a sitemap, with a shared Worker thread in each iframe phoning home to tell the controller when it has finished loading so the next URL can be requested. That would allow for any number of browser based checkers (e.g. AXE core) to run without hitting the server. But oh goodness the load it would create, as every request would be uncached and include JSON hits with the result.

🇺🇸United States itmaybejj

It can be done for sure -- just make AJAX calls and report errors...but it would be less effective and more work in my opinion -- links often go stale on pages you are not viewing, so link checking is usually done by crawling rather than live checking. And then to keep the load down on the server and the browser, I'd want to create a table of every found link and when it was last checked...

🇺🇸United States itmaybejj

Oh I'll also say since you mentioned SI -- I am also looking into doing this using the API from our third-party solution. E.g., piping the broken links it found into the Editoria11y interface, so editors can view all their issues from any source via Editoria11y's UI.

Ideally I would love to just have a bucket of connectors in the module -- LinkChecker, SiteImprove, DubBot, etc. There would need to be a lot of filtering though -- a lot of issues from the "bigger" vendors are irrelevant to content editors. So I'd only want to pipe in a few relevant tests.

🇺🇸United States itmaybejj

Longer answer sent privately, but for the record:

For Drupal, I would want to filter it to relevant things for the current page, like I do for the dismissals on the page.
That array is just slapped into drupalSettings at line 343.

For broken links, the first thing I would try is building a selector – performance testing would be needed of course. E.g., ‘a[href=”example1”], a[href=”example1”]’. And then pass that to the native finder function:

Ed11y.findElements('error404','a[href="example1"], a[href="example2"]');
  Ed11y.findElements('error403','a[href="example3"], a[href="example4"]');

The rest would follow the pattern from the safeLinks test -- add each of the found items to the results array, and provide the various strings needed to build each type of tip.

Honestly – the Editoria11y side of this is really easy for me. The hard part for me would be rooting around in LinkChecker’s tables to shove the list of broken URLs on a route into drupalSettings somewhere. If someone took that on, I could do the rest.

🇺🇸United States itmaybejj

Much progress! 1-3 are done, and yes some users have already used them to write connectors for the library in other CMSs.

I definitely will not be writing my own link checker. It's just too far outside my expertise, and my employer already pays for a vendor tool.

The next thing I'm going to work on is rewriting my database tables to have more robust node references. If I get that working it should make connecting with other modules via the dashboard initiative much easier.

It will probably take me a year to write all that myself, but the pieces are in place for someone to contribute connectors with specific modules quite easily before then if they have the time; I'd be happy to show them how to do it, and I have some code samples users have sent to me over the years.

🇺🇸United States itmaybejj

Me too. Unfortunately I do not think it is possible with the current database tables using Views -- I originally implemented my "Entity ID" field in a way that conflated NID and TID. It is my hope to write some hooks to rewrite my tables and make this possible later this year as part of 3492299: Add full views integration for the content dashboard Add full views integration for the content dashboard Postponed .

I do think a custom entity query could do it in the interim, by writing a JOIN against the node and Editoria11y tables, and filtering the Editoria11y tables to only include nodes.

🇺🇸United States itmaybejj

Looks like 2.2.3 is up and ready with all the new strings. Is it ok for me to mark this fixed?

🇺🇸United States itmaybejj

Please test 2.2.3. If you type "Ed11y" in the console you can see if the library is loaded. It should not be present on the Layout tab.

If your editing environment loads Layout Builder forms on other routes, it would help me to know what the route names are, and/or what CSS selector reliably detects your forms.

🇺🇸United States itmaybejj

I realized -- even though I was not initializing the library on Layout Builder editor routes, I was still attaching the library. It doesn't do any harm to be there if the auto-disablement works, but it's needless. This commit will prevent it from even attaching to those routes.

🇺🇸United States itmaybejj

I'll keep this open until l.d.o gets the new strings then. I'm assuming there's nothing I can do at my end to help with that?

🇺🇸United States itmaybejj

That's...profoundly troubling. Editoria11y is supposed to disable itself if it detects the Layout Builder editor interface is present on the page (.layout-builder-form) -- I learned during the beta that Layout Builder has scroll listeners that make it incompatible with another module modifying the page markup -- it results in what you describe. Something is very wrong if Editoria11y is running while Layout Builder is open.

Could you tell me what Editoria11y version you are on, what version of Drupal core you are on, and, if you are up for checking your browser console, whether anything is found if you type document.querySelector('.layout-builder-form')?

🇺🇸United States itmaybejj

Oh -- so those placeholders are gone now, so it is just outputting %headingExample as plain text. The text that used to be %headingExample is visible in your screenshot -- it is the nested bullets.

I don't actually see the new strings on the translation server yet. That example should now be made of 4 separate strings, with no placeholders:

headingExample : `
    <ul>
        <li>${Drupal.t("Heading level 1")}
            <ul>
                <li>${Drupal.t("Heading level 2: a topic")}
                    <ul>
                        <li>${Drupal.t("Heading level 3: a subtopic")}</li>
                    </ul>
                </li>
                <li>${Drupal.t("Heading level 2: a new topic")}</li>
            </ul>
        </li>
    </ul>`
  ,
headingEmpty : {
    title: Drupal.t("Heading tag without any text"),
    tip: () => `
      <p>
      ${Drupal.t("Headings and subheadings create a navigable table of contents for assistive devices. The numbers indicate indents in a nesting relationship:")}
      </p>
      ${Ed11y.M.headingExample}
      <p>${Drupal.t("Empty headings create confusing gaps in this outline: they could mean the following content is still part of the previous section, or that the text was unpronounceable for some reason.")}</p>
      <p>
        ${Drupal.t("<strong>To fix:</strong> add text to this heading, or delete this empty line.")}
      </p>`,
  },
🇺🇸United States itmaybejj

Hopefully all fixed in 2.2.1 , and I added a debug parameter to make it easier to review in the future.

🇺🇸United States itmaybejj

I'm going to do both-- I'm reformatting the strings to support Drupal.t() for all languages. But people also use the library on other CMS platforms. And any languages I have natively supported in the JS library I don't have to pass through Drupal.t().

I hope to finish today or tomorrow.

🇺🇸United States itmaybejj

I will keep working on breaking up the strings then, since I know that will work.

The other translation option is to add a DE object to the library languages file. That would be much more robust between versions, and I would then just use the library translation rather than Drupal.t for the JavaScript strings.

🇺🇸United States itmaybejj

Question -- did you have this problem in 2.1? I could revert most of these strings to what they were in 2.1 for now, which would restore existing translation work.

🇺🇸United States itmaybejj

Alright; I wrote some tests to compare strings and confirmed that most of the missing strings are due to HTML tags in the strings. This is definitely going to take me a few days to fix.

🇺🇸United States itmaybejj

The good news is I can reproduce the issue.

The bad news is that I do not understand where the problem is coming from. These strings are available for translation, but I think I may have HTML in these strings that is causing the Drupal.t() function to ignore them.

I hope to have a fix to you this week, but I am concerned I may need to break up a lot of strings into individual sentences, which will be very time consuming and break a lot of existing translation work.

Production build 0.71.5 2024