Use "legend" HTML element instead of label in 'captcha.html.twig', use div if no title given

Done, please review! See screenshot:

We made that change intentionally, as label's have Drupal default styling, while fieldset legend's don't.
See the issue over here: 📌 Use label & description for captcha label & description instead of custom structure Fixed I'm not very motivated to reverse that decision.

But another point was also raised:
@thomas.frobieter mentioned that for the fieldset styling should have further classes for core themes, eventually.

Also, changing the markup might break existing installations (BC), so we have to double-check if this is still allowed in beta.

Assigning @thomas.frobieter to add possible Drupal Core default classes (that won't break things?).

Once 2.0.0 is released, we'll definitely not be allowed to make such changes anymore for a widely used module like this:

Betas for minor and major versions

The goal of the beta phase is to prepare a polished, stable release, and to provide a testing target for theme or module developers and site owners.

We restrict changes during the beta phase in order to reduce disruptions and manage technical debt. New features and APIs must be targeted for the next development minor instead, and public APIs must remain stable.

The release of the first beta is a firm deadline for all feature and API additions. Even if an issue is pending in the RTBC queue when the commit freeze for the beta begins, it will be committed to the next minor release only.

The following types of changes are allowed during the beta phase:

• criticals
• bug fixes and contributed project blockers [19 issues], if they are non-disruptive, or if the impact outweighs the disruption
• API documentation improvements
• patch- and minor-level library updates
• string changes
• user interface changes to fix usability or accessibility issues, if the impact outweighs the disruption
• minor API additions or internal API changes to fix bugs, if the impact outweighs the disruption
• changes requiring an upgrade path, if the impact outweighs the disruption
• Certain other issues with high impact and low disruption at committer discretion only.

https://www.drupal.org/about/core/policies/core-change-policies/allowed-... →

A good reason for using the legend would be, that some of the captcha implementations use their own label for the captcha input. Others don't.

While our label will never technically collide with them, using legend would 100% separate them in all cases.

I think best will be to compare the outcome in major Drupal themes, and afterwards @thomas.frobieter should have the last word on that. These should be our final (justified breaking) changes to the twig file anyway until 3.x!

Setting this to major, as it prevents the 2.x release and enables the module for 📌 Release 2.0.0 to safe folks from wrong versions Closed: duplicate to apply.

So, the idea is:
- Use the regular fieldset renderer, so the fieldset.html.twig will be used
- Use the captcha.html.twig only for the fieldset contents

Both are overridable by themes, while getting the default fieldset styles from the theme. So we don't get a ugly unstyled fieldset, like we currently do in for example Claro and Olivero.

This way, the decision to use legend or label ist not up to us, its up the the theme and thats fine.

Re #9 ok so let's test this here in the MR, see what the results and pro / con's are and if it takes too much time / isn't worth it. Based on that, we should then decide, how to proceed further. From simple to complex.

Nice ideas for improvement @thomas.frobieter. Anyway, I think we shouldn't invest days and set a justified limit.

I agree, so @Grevil can you please figure out if we are able to use the regular fieldsets, etc. like described in #9? I'll help with the templates, classes etc. if required.

One more point: We need to ensure that we have our own theme suggestion just for this fieldset render!

As we don't want to add one for any fieldset, we need to pass it in the render array itself. This seems possible, see: https://www.drupaladventures.com/article/theme-suggestions-theme-render-...

Okay... I just wanted to give this a first shot, went into the code and remembered @Grevil's and my pain around alll these structures... And honestly, I don't think the proposed change to fieldsets is possible within hours, not even a day, I think. And it's very risky, as then the fieldset will influence the captcha's array structure technically (the old tree thing...)

I would have tried it, because I see the benefits from what @thomas.frobieter said, but this should be done with or after CAPTCHA rewrite from scratch. This is far above our heads and budgets.

@thomas.frobieter may still decide to make the switch back to legend instead of label, I won't block that, if there are good enough reasons and if it's before 2.0.0.

Someone always has to pay...

Flip a coin, I don't think its really relevant. Label is better to style, Legend is semantically correct.
Some sites already use the beta release, so they have might already styled it as label. So, lets stick with this.

Ok, I guess we can set this postponed to whenever and do a real release?

Not sure what would be postponing work on this so unblocking.

Relevant WCAG rule this would fix. https://www.w3.org/WAI/WCAG21/Techniques/html/H71

Good point @neclimdul!

The first element inside the fieldset must be a legend element, which provides a label or description for the group.

https://www.w3.org/WAI/WCAG21/Techniques/html/H71

So let's fix the tests and do this. Who's willing?

@neclimdul what do you think about this proposed change? That would also comply with the default and solve issues with 🐛 Google Tag manager is loaded without consent Fixed , if no title is used.

@neclimdul what do you think about this proposed change? That would also comply with the default and solve issues with 🐛 Google Tag manager is loaded without consent Fixed , if no title is used.

Re-introduced backwards-compatibility for the structure, if no title is set and if a title is set, improved the markup.
This was planned to be 2.x only, but with ✨ Difficult migration from Drupal 9 to 10 Fixed got into 8.x-1.* which is out of scope here.

Nice, tests are all green! RTBC!

Thanks @Grevil, let's merge this into 2.x and keep it there for some days. Also fixing 🐛 8.x-1.12 shows captcha fieldset, even without captcha output Fixed by bringing back the old structure, if no title is given.

Automatically closed - issue fixed for 2 weeks with no activity.