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

Merge Requests

Recent comments

🇺🇸United States aaronpinero

My first thought is to add a validation function to the replacement form checking to see if the tid of the existing term is the same as the tid of the new (replacement) term. If they are the same, return an error.

🇺🇸United States aaronpinero

aaronpinero made their first commit to this issue’s fork.

🇺🇸United States aaronpinero

Hi Peter,

Yes, I have a working module that I use for one website that I manage. The module still has the limitations I described in the original post, but it works for my limited needs, so I've not had the motivation to work on any of the proposed enhancements.

🇺🇸United States aaronpinero

Thank you! I took a closer look at the original Sally code myself just to see what's going on here I think I understand why some of this was done the way it was done. I appreciate the effort to make it possible to run this with a CSP.

🇺🇸United States aaronpinero

@mandclu making this configurable makes sense. I will work on that.

🇺🇸United States aaronpinero

Thank you @cboyden for the 10.2.x patch. I have applied this to my v10.2.5 Drupal site using composer. The patch applied cleanly and appears to provide the expected a11y enhancement.

🇺🇸United States aaronpinero

I created a patch for testing this by outputting a diff as follows:

% git diff 2.1.0-alpha1 3314213-add-open-by > ckeditor_details-3314213.patch

The problem with the current MR is that the merge request is for merging into 2.x (or maybe 2.1.x) and so the plain diff of the merge won't work for patching if you've installed the 2.1.0-alpha1 release of the module. The attached file worked for me when applied via composer where the plain diff of the MR did not. This should allow folks to test this change by patching the current module release.

One note: after patching the module, I found it necessary to re-save the text input format that is using the details widget. For some reason, the open attribute of the details element doesn't get immediately added to the allowed tags for the input format. After saving (with no changes) it was added and then the option to be open by default was available.

This worked for me on Drupal 10.2.5 using CKEditor 5 and CKEditor Detail Accordion 2.1.0-alpha1 patched with the attached patch via composer.

🇺🇸United States aaronpinero

I can also confirm that patch#21 seems to solve the problem in Drupal 10.2.5

🇺🇸United States aaronpinero

I also appreciate having a patch available. I tested the patch in #77 and it worked for me in Drupal 10.2.4. I understand the desire to have folks follow the current practice of using a merge request, but it is also convenient to have the patch. I think we can encourage the following of desired practice without discouraging helpful contributions.

🇺🇸United States aaronpinero

Nevermind, this does seem to work, sort of. It's just not documented anywhere how to do it. I think I've figured out how to make it work.

🇺🇸United States aaronpinero

With more experimentation, I realize now that facet order cannot be set at all through the facets admin interface. No matter what, the order of facets in the endpoint response seems to be alphabetical by key name. It would be nice if (a) the behavior was consistent and (b) the order of facets in the endpoint response respected the facet admin weight settings.

🇺🇸United States aaronpinero

@jacobupal TBH I never actually tried applying the change as a patch. I had only been testing the modified version of the module. However, you are correct. I also was unable to apply the patch.

If I had to guess, the part of the patch that's the problem is the change to js/build/detail.js. The change is essentially a replacement of the existing file. However, I suspect patch is trying to identify the existing line of code (one huge line in a minified JS file) and replace it with the new line of code (another huge line in a minified JS file). There could be little trivial differences in the minified code that are functionally insignificant but are throwing off the diff. Or maybe that one line of minified code is just too long.

Not sure how to resolve that one. It would be better to just get this merged and have a new release. But obviously some RTBC needs to be done first.

🇺🇸United States aaronpinero

@acbramley I wanted to make sure I understand what you were reporting in #55 and #59. This is important to me because this issue is blocking the upgrade of my Drupal websites to 10.2. I am not going to tell my content editors that I now have to take away what is really a basic, essential feature of the content editor.

It sounds like you were saying that, following the upgrade to Drupal 10.2, I will need to:

- re-save my text format configurations
- export the website configuration to xml
- manually update the editor.editor.x.yml files to set the list properties.styles to true and add

    and
    to the allowed tags
    - import the configuration

    It also sounds like I will need to repeat this process if, in the future, I need to change my text format configurations for some other reason besides list styles. Is that correct? Are there any other steps that are necessary to get this to work? Is there a new patch I need to apply or are these steps sufficient without a patch? Thanks for working through this.

    ---------

    I very much hope some of this can be resolved via patch before 10.3. This is really basic functionality for the editor that should never have been missing in the initial release of 10.0. More than a year later, it would be nice to see this finally cleaned up.

🇺🇸United States aaronpinero

I can see where someone might not want an unpublished or draft node to be updated. Perhaps there should be a user option on the Replace page for the user to indicate if unpublished nodes should be included/excluded from the replace.

🇺🇸United States aaronpinero

Thank you @malcomio. This is great news.

🇺🇸United States aaronpinero

Hi all. I would definitely like to move this issue along. It's been indicated that tests are needed to move this back to "needs review". I would like to help but I am a novice developer and the only tests I've ever written for a module were based heavily on provided examples of Unit and Kernel tests. I don't know a whole lot about selecting the appropriate tests and then writing them.

That said, I did a bit of digging into the media module code. For tests related to the thumbnail formatter (where the proposed patch is applied) there is a Kernel test (core/modules/media/tests/src/Kernel/MediaThumbnailFormatterTest.php) and a Functional test (core/modules/media/tests/src/Functional/FieldFormatter/MediaThumbnailFormatterTest.php). The Kernel test seems to provide only a test to verify settings of the field formatter. The Functional test appears to actually test the rendering of the formatted media field as it would be used in a node.

Based on what I'm reading in the code, the appropriate test addition for this change to the module would be to modify the Functional test that already exists to include some check of the rendered alt text vs the alt text stored in the reference media entity. Leaving aside (for the moment) the details of actually executing this change to the functional test, does this seem like a reasonable plan? Are there additional tests that would need to be included that I haven't considered?

🇺🇸United States aaronpinero

Thank you everyone who took the time to review this. I appreciate it.

🇺🇸United States aaronpinero

Okay, I have provided an updated patch that, to reset a sort view, executes the deletion of rows from the db table but doesn't write any new data. I think this is more in keeping with the solution proposed in the original issue.

🇺🇸United States aaronpinero

I realize I provided an incorrect solution. I will update the patch so, to reset, the table rows are not written to the database in the reset handler.

🇺🇸United States aaronpinero

I am adding a patch which seems to, at least in my limited manual testing, provide the solution. I updated the .module file to add a draggableviews_views_reset handler function. This function is a near exact copy of the draggableviews_views_submit function, except all the weight values are set to 0 when data is written to the database. I also updated the draggableviews_form_alter to add the reset button.

It would be great if a maintainer could take a look at this. I think this is an easy and sensible addition to the module.

🇺🇸United States aaronpinero

@deborahblessy Thank you for your suggestion regarding loading the jQuery library from CDN vs. requiring the library in composer. That might have fixed the issue. I need to do a more thorough check, but a quick test seems to indicate the console errors are gone.

The original problem description had some typos because I copied the text from a screen capture. I guess OCR still has issues in 2023.

🇺🇸United States aaronpinero

I tried applying patch #43 to the beta6 release via composer. The patch applied cleanly, but my pages using the views reference field displayed the WSOD. The log recorded the following error:

AssertionError: When a #lazy_builder callback is specified, no properties can exist; all properties must be generated by the #lazy_builder callback. You specified the following properties: #attached. in assert() (line 334 of /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php)
#0 /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php(334): assert(false, 'When a #lazy_bu...')
#1 /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php(204): Drupal\Core\Render\Renderer->doRender(Array, false)
#2 /var/www/html/web/core/modules/views/src/Plugin/views/field/EntityField.php(934): Drupal\Core\Render\Renderer->render(Array)
#3 /var/www/html/web/core/modules/views/src/Plugin/views/field/FieldPluginBase.php(1170): Drupal\views\Plugin\views\field\EntityField->render_item(0, Array)
#4 /var/www/html/web/core/modules/views/views.theme.inc(238): Drupal\views\Plugin\views\field\FieldPluginBase->advancedRender(Object(Drupal\views\ResultRow))
#5 [internal function]: template_preprocess_views_view_field(Array, 'views_view_fiel...', Array)
#6 /var/www/html/web/core/lib/Drupal/Core/Theme/ThemeManager.php(287): call_user_func_array('template_prepro...', Array)
#7 /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php(433): Drupal\Core\Theme\ThemeManager->render('views_view_fiel...', Array)
#8 /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php(204): Drupal\Core\Render\Renderer->doRender(Array, false)
#9 /var/www/html/web/core/modules/views/src/Plugin/views/field/FieldPluginBase.php(1746): Drupal\Core\Render\Renderer->render(Array)
#10 /var/www/html/web/core/modules/views/src/Plugin/views/style/StylePluginBase.php(777): Drupal\views\Plugin\views\field\FieldPluginBase->theme(Object(Drupal\views\ResultRow))
#11 [internal function]: Drupal\views\Plugin\views\style\StylePluginBase->elementPreRenderRow(Array)
#12 /var/www/html/web/core/lib/Drupal/Core/Security/DoTrustedCallbackTrait.php(101): call_user_func_array(Array, Array)
#13 /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php(788): Drupal\Core\Render\Renderer->doTrustedCallback(Array, Array, 'Render #pre_ren...', 'exception', 'Drupal\\Core\\Ren...')
#14 /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php(374): Drupal\Core\Render\Renderer->doCallback('#pre_render', Array, Array)
#15 /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php(204): Drupal\Core\Render\Renderer->doRender(Array, false)
#16 /var/www/html/web/core/modules/views/src/Plugin/views/style/StylePluginBase.php(716): Drupal\Core\Render\Renderer->render(Array)
#17 /var/www/html/web/core/modules/views/src/Plugin/views/style/StylePluginBase.php(582): Drupal\views\Plugin\views\style\StylePluginBase->renderFields(Array)
#18 /var/www/html/web/core/modules/views/src/Plugin/views/style/StylePluginBase.php(473): Drupal\views\Plugin\views\style\StylePluginBase->renderGrouping(Array, Array, true)
#19 /var/www/html/web/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php(2169): Drupal\views\Plugin\views\style\StylePluginBase->render(Array)
#20 /var/www/html/web/core/modules/views/src/ViewExecutable.php(1538): Drupal\views\Plugin\views\display\DisplayPluginBase->render()
#21 /var/www/html/web/core/modules/views/src/Plugin/views/display/Block.php(131): Drupal\views\ViewExecutable->render()
#22 /var/www/html/web/core/modules/views/src/ViewExecutable.php(1635): Drupal\views\Plugin\views\display\Block->execute()
#23 /var/www/html/web/core/modules/views/src/Element/View.php(81): Drupal\views\ViewExecutable->executeDisplay('block_3', Array)
#24 /var/www/html/web/core/modules/views/src/Plugin/Block/ViewsBlock.php(59): Drupal\views\Element\View::preRenderViewElement(Array)
#25 /var/www/html/web/core/modules/block/src/BlockViewBuilder.php(171): Drupal\views\Plugin\Block\ViewsBlock->build()
#26 [internal function]: Drupal\block\BlockViewBuilder::preRender(Array)
#27 /var/www/html/web/core/lib/Drupal/Core/Security/DoTrustedCallbackTrait.php(101): call_user_func_array(Array, Array)
#28 /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php(788): Drupal\Core\Render\Renderer->doTrustedCallback(Array, Array, 'Render #pre_ren...', 'exception', 'Drupal\\Core\\Ren...')
#29 /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php(374): Drupal\Core\Render\Renderer->doCallback('#pre_render', Array, Array)
#30 /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php(446): Drupal\Core\Render\Renderer->doRender(Array)
#31 /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php(204): Drupal\Core\Render\Renderer->doRender(Array, false)
#32 /var/www/html/web/core/lib/Drupal/Core/Template/TwigExtension.php(479): Drupal\Core\Render\Renderer->render(Array)
#33 /var/www/html/web/sites/default/files/php/twig/64f8e0a2e7225_page.html.twig_e7hh4y0wifva4a_ScRFDQ36Sd/zfFVEjIYHqdRX1anIHqxRCGpcFxi4utbCkf16f__ry0.php(147): Drupal\Core\Template\TwigExtension->escapeFilter(Object(Drupal\Core\Template\TwigEnvironment), Array, 'html', NULL, true)
#34 /var/www/html/vendor/twig/twig/src/Template.php(405): __TwigTemplate_4c484e285e34c1c517dfce0f56403aae->doDisplay(Array, Array)
#35 /var/www/html/vendor/twig/twig/src/Template.php(378): Twig\Template->displayWithErrorHandling(Array, Array)
#36 /var/www/html/vendor/twig/twig/src/Template.php(390): Twig\Template->display(Array)
#37 /var/www/html/web/core/themes/engines/twig/twig.engine(55): Twig\Template->render(Array)
#38 /var/www/html/web/core/lib/Drupal/Core/Theme/ThemeManager.php(384): twig_render_template('themes/custom/h...', Array)
#39 /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php(433): Drupal\Core\Theme\ThemeManager->render('page', Array)
#40 /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php(204): Drupal\Core\Render\Renderer->doRender(Array, false)
#41 /var/www/html/web/core/lib/Drupal/Core/Template/TwigExtension.php(479): Drupal\Core\Render\Renderer->render(Array)
#42 /var/www/html/web/sites/default/files/php/twig/64f8e0a2e7225_html.html.twig_K7e0YUqNYNYFXZjaSejrhWCNJ/F9GmlgEUaQj5BFA8BJiV1mhKLHdT20aW1lYKJfOlhzk.php(87): Drupal\Core\Template\TwigExtension->escapeFilter(Object(Drupal\Core\Template\TwigEnvironment), Array, 'html', NULL, true)
#43 /var/www/html/vendor/twig/twig/src/Template.php(405): __TwigTemplate_e64b960fb8704841d8453d0d3f187c3c->doDisplay(Array, Array)
#44 /var/www/html/vendor/twig/twig/src/Template.php(378): Twig\Template->displayWithErrorHandling(Array, Array)
#45 /var/www/html/vendor/twig/twig/src/Template.php(390): Twig\Template->display(Array)
#46 /var/www/html/web/core/themes/engines/twig/twig.engine(55): Twig\Template->render(Array)
#47 /var/www/html/web/core/lib/Drupal/Core/Theme/ThemeManager.php(384): twig_render_template('themes/custom/h...', Array)
#48 /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php(433): Drupal\Core\Theme\ThemeManager->render('html', Array)
#49 /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php(204): Drupal\Core\Render\Renderer->doRender(Array, false)
#50 /var/www/html/web/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php(162): Drupal\Core\Render\Renderer->render(Array)
#51 /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php(580): Drupal\Core\Render\MainContent\HtmlRenderer->Drupal\Core\Render\MainContent\{closure}()
#52 /var/www/html/web/core/lib/Drupal/Core/Render/MainContent/HtmlRenderer.php(163): Drupal\Core\Render\Renderer->executeInRenderContext(Object(Drupal\Core\Render\RenderContext), Object(Closure))
#53 /var/www/html/web/core/lib/Drupal/Core/EventSubscriber/MainContentViewSubscriber.php(90): Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Routing\CurrentRouteMatch))
#54 [internal function]: Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object(Symfony\Component\HttpKernel\Event\ViewEvent), 'kernel.view', Object(Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher))
#55 /var/www/html/web/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php(142): call_user_func(Array, Object(Symfony\Component\HttpKernel\Event\ViewEvent), 'kernel.view', Object(Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher))
#56 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(174): Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object(Symfony\Component\HttpKernel\Event\ViewEvent), 'kernel.view')
#57 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(81): Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request), 1)
#58 /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/Session.php(58): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#59 /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(48): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#60 /var/www/html/web/core/modules/ban/src/BanMiddleware.php(50): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#61 /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(48): Drupal\ban\BanMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#62 /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(51): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#63 /var/www/html/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#64 /var/www/html/web/core/lib/Drupal/Core/DrupalKernel.php(718): Stack\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#65 /var/www/html/web/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request))
#66 {main}

🇺🇸United States aaronpinero

@Nishant2312 based on your description, I do not think you configured your menu correctly to illustrate the problem. Perhaps this could be better explained over a screen sharing session on Slack. I would be happy to schedule something if you feel it would be helpful.

🇺🇸United States aaronpinero

Thank you @hemangi-gokhale for the specific recommendations. I have addressed the points listed in your message and updated the 1.0.x branch accordingly.

🇺🇸United States aaronpinero

@Shiraz_Dindar this is not correct. The changes appear to have been merged into the -dev branch, but they are not in the tagged 3.11 release. There needs to be a new tagged release (3.12?) that incorporates the changes.

🇺🇸United States aaronpinero

The patch in #16 applied cleanly using composer, fixed the compatibility of the module with D10 (as per upgrade status module) and seems to preserve the functionality of the module. So RTBC+1

🇺🇸United States aaronpinero

Patch #34 does not cleanly apply to the latest beta (6). I didn't check #40. I've been depending on patch #34 for quite a while now, it would be great if we could get an updated patch (or, better yet, something permanent merged).

Thanks.

🇺🇸United States aaronpinero

I may have been successful in adding UI for the CKEditor Details widget allowing users to set the default open state of the details element.

With the update to the module, if the user clicks on the details widget in the editor, the user will see a balloon popup with a switch button. The switch button is labeled "open by default" and the switch is default off. If the user toggles the switch, the details element will be open by default when the content is viewed.

The balloon popup will not be displayed when the user is editing the summary or text within the details wrapper element. The user needs to click on the grey area surrounding the summary and wrapper for the balloon to appear.

🇺🇸United States aaronpinero

Since there hasn't been any response to this request in many months, I am trying to do it myself.

The update I am looking to make will expose a toggle switch when a details widget is selected in the editor, allowing the using to indicate if the details widget should be open by default or not.

If anyone's interested, I am working on the CKEditor part of this update outside of Drupal. This allows me to focus on the functionality of the CKEditor plugin without worrying about the Drupal integration. The code-in-progress is at https://github.com/aaronpinero/cke5_dev

🇺🇸United States aaronpinero

Okay, the dev branch now includes the update to the ContentReviewNotificationSettingsForm to use dependency injection of the State.

🇺🇸United States aaronpinero

Can we get this merged and a new release created? Looking at this patch, the update was simply to change the supported Drupal versions in the .info.yml file -- a truly benign update.

🇺🇸United States aaronpinero

@vinaymahale the line exceeds 80 characters issue has been resolved on the 1.0.x branch, along with several other similar error notices.

🇺🇸United States aaronpinero

@scott_euser thank you for this suggestion. Since there was only one place where I was using the Drupal::messenger() service, I removed it.

I also adjusted the two main functions in the .module file (the hook_cron() and simple_content_notifications_prep()) to perform a check for the enabled status configuration value before doing anything else. This way nothing happens if that part of the module function is not enabled by the administrator.

To help make this more clear, I also adjusted the config forms so that the enable/disable option is the first item in each configuration form.

The changes are committed to the dev branch.

🇺🇸United States aaronpinero

Can a new release be created that includes the D10 compatibility updates?

🇺🇸United States aaronpinero

@scott_euser I am thinking that I might, while working on the other issue, split this into two configuration options so logging for each type of notification (content updates vs content review) can be enabled/disabled separately. Right now, the option makes it seem like this option is just for logging with content update notifications.

🇺🇸United States aaronpinero

I've reviewed the patch and it mostly looks good. There are two things that I need to address:

- there is call to the config service in the hook_cron function that can be removed since the date is going to now be stored in state.
- there needs to be accounting for the case where there is no saved state.

🇺🇸United States aaronpinero

@scott_euser thank you for this work. I will do my best to review this today or tomorrow so as to get the module updated quickly.

🇺🇸United States aaronpinero

This issue has been resolved in the latest release.

🇺🇸United States aaronpinero

Setting to needs review to get some other eyes on this.

🇺🇸United States aaronpinero

Existing tests for the StringFilter in the Views module all have to do with whether or not use of the exposed filter produces the expected result set. I don't know what sort of assertion one might use to to test this change other than adding a very simple test that checks to see if the input string longer than 128 characters is truncated. This is easy enough to test manually, so I don't know that it's necessary to add an automated test.

If no one has a differing viewpoint, this could be RTBC.

🇺🇸United States aaronpinero

As suggested in #7 and #10, I have created an updated version of the patch that has comments.

🇺🇸United States aaronpinero

The parent issue has proposed a fix that would address this issue "further up the chain" from Views. However, that fix is against Drupal 11.x, so we do need a fix applied for Drupal 9.5 (as this will be around for a while yet). The parent issue also suggests (option b) to set the maxlength to to 256 instead of 128 and we should probably go with that, providing a fix against Drupal 9.5.x

🇺🇸United States aaronpinero

Looking at this issue, it seems like the following specific tasks will help get it to RTBC:

1. rewrite of the issue summary using the standard template. Also, generalizing the summary since this is about a search input but the character limit affects views generally.

2. agreeing on the patch that sets maxlength to 255. This seems to be a responsible setting (as opposed to null) and matches the aforementioned title character limit.

3. Check to see what tests are available for this module and if tests are adequate or need to be updated.

These notes are mostly for #novice #contribution #DrupalConPittsburgh2023

🇺🇸United States aaronpinero

@vishal.kadam I addressed the items you listed.

For the info.yml file, I set the core version requirement to ^9 || ^10. The addition of ^10 is in an open issue on the module for automated D10 compatibility fixes. The addition of ^10 to the core version requirement was the only compatibility fix listed.

I also removed the unused methods from the Forms. I was previously under the impression that these methods needed to be declared in order for the Form controller to work, regardless of whether it was used. However, I see in manual testing that the forms work fine without the unused method being declared.

🇺🇸United States aaronpinero

I am uploading a new patch because the current one doesn't apply cleanly through composer with Drupal 9.5.9. There was an update in 9.5.9 to the media thumbnail formatter that need to be accounted for. That said, the patch is VERY similar.

🇺🇸United States aaronpinero

One thing I noted about the site after the patch: if the image is missing alt text, an empty alt attribute is added to the image. Not sure if that's a desired outcome... would it be better to leave off the alt attribute entirely if there's no text available?

🇺🇸United States aaronpinero

I tested the patch with Drupal 9.5.8 and it appears to solve the issue. I am seeing alt attribute information on a media image thumbnail where previously there was none.

🇺🇸United States aaronpinero

This seems like a pretty basic and important accessibility problem. I just noticed it today when doing a WAVE report on my website. This issue was the first error reported. I thought it was a content author mistake. It's a surprising oversight for the Media system. Can we bump this up to critical?

🇺🇸United States aaronpinero

A new release has been created which includes the update.

🇺🇸United States aaronpinero

Thanks for adding this. I will work on getting the change merged in.

🇺🇸United States aaronpinero

Uploading new patch with previously mentioned accessibility recommendations.

🇺🇸United States aaronpinero

I wonder if it would be useful to follow this accessibility guidance:
https://a11y-guidelines.orange.com/en/articles/technical-iframe/

I would consider the google tag manager iframe to be a technical iframe (as opposed to an iframe that is providing content to the user. For accessibility compliance and for hiding the iframe from the accessibility API, it is recommended that:

- the iframe has a title that identifies it as a technical element rather than a content element
- it has the attribute aria-hidden="true"
- it has tabindex="-1"

🇺🇸United States aaronpinero

My apologies for any confusion. I say "attempted" because I was attempting to resolve the issue by updating from 6.0.0-beta4 to 6.0.0-rc1. I use composer to manage module updates. There wasn't any error with the update, and the module appears to function properly.

The issue was (and remains) the fact that the module is appearing in reports (in the Drupal admin) as Not Compatible and version 6.1.0-rc1 is listed as the recommended version even though I am not on Drupal 10. From what I understand from the module page, version 6.0.0-rc1 should be the correct, recommended version since I am running Drupal 9.5.

In my case, the module should not be listed as Not Compatible and version 6.1.0-rc1 should not be recommended.

The core_version_requirement in the module's .info file is correct, so I don't know what the source of the issue is.

🇺🇸United States aaronpinero

UPDATE: I am testing the v2.1 alpha version of the module with CKEditor 5. Unlike in CKEditor 4 / v2.0.3 where the open state of the details widget in the editor was preserved a node was published, all detail widgets created with CKEditor 5 are default closed, regardless of the open state in the editor. I would like to see the previous behavior preserved to give content authors the option to have the details widget default to open.

🇺🇸United States aaronpinero

The original patch posted by @icurk didn't apply cleanly for me (I'm using Drupal 9.5.3) but I was able to update the patch to work. I will try to add this to the issue. The patch appears to resolve the issue for me in that the menu respects the unchecked "show as expanded" option even when the option is the currently selected option.

🇺🇸United States aaronpinero

I'd like to see this issue reopened. I don't believe the original issue was solved in that, while the system may work as designed, it does not work as desired. The problem is that, the way it works now, menu option appears expanded when it is the currently selected item even if you have unchecked the "show as expanded" option in the menu link configuration. This is true even if in your menu block configuration you have unchecked the "expand all menu options" option. The only way to keep the selected menu option from showing as expanded is to restrict the menu levels. This makes it impossible to have a menu where you want one option to expand but another option to not expand.

Here is my specific case:

I have a main menu. Two of the options in the main menu have child menu options. I want one to expand and the other to not expand. In the menu configuration, I checked "show as expanded" for the first menu option and left this unchecked for the second menu option.

For my menu block, I have configured it to show 2 levels to display and I have left the expand all menu links unchecked.

The result is that everything works fine except when the second menu option is the select menu option. In that case, even though I have not configured the option to show as expanded, it shows as expanded anyway because it's the selected option.

What I want is for this option to never show as expanded in this block, even if it's selected.

This could be done by respecting the existing "show as expanded" configuration. Or, perhaps there could be a menu setting that determines if selected menu options appear expanded.

Production build 0.69.0 2024