Adding attributes to views-view-list.html.twig doesn't work if Views List class Style option is empty

Yeah I've run into this in other parts of core too, annoying to have to do a hook_preprocess to just add the Attribute. I did a quick scan of the rest of views.theme.inc and this seems the only instance where this seems relevant.

Never looked into the testing side of theme stuff, can we/should we test this?

Rerolled the patch.

It also works great! RTBC

No one's answered @Lendude's question about whether this should be tested. It would be good at least to know if we have exiting test coverage of adding a class before making this change.

Hi, just randomly found this issue while looking at views module issues, and thought I'd chip in.
If this is happening on other parts of core, I wonder whether we should do the test for all of core templates, which would help us identify every instance where this is happening. I'm guessing we should perhaps be patching template_preprocess() ? Should we open a follow up?

I'd like to take this one step further and only instantiate the Attribute if it hasn't already been created somewhere else.

It's pretty complicated for mere mortals to guarantee the order in which preprocess functions run, so if someone has setup Attributes in another preprocess for a contrib/custom module or theme, they might be blown away by this.

Wrapping the statement in an if or ternary operator could fix this:

if (empty($variables['list']['attributes']) {
  $variables['list']['attributes'] = new Attribute();
}

Would be nice to standardize on this as the preprocess way throughout core/contrib.

In the same vein, if you're just trying to add classes in the twig template, you can make use of the create_attribute() twig command, shown here in the context of the list.attributes in a custom views-view-list.html.twig using ternary operator:

{% set list_attributes = (list.attributes) ?: create_attribute() %}
{% set list_attributes = list_attributes.addClass('my-list-element-class') %}

Or the long hand form of the above:

{% set list_attributes = list.attributes %}
{% if list_attributes is empty %}
{% set list_attributes = create_attribute() %}
{% endif %}
{% set list_attributes = list_attributes.addClass('my-list-element-class') %}

Can confirm that this patch does not apply, but is absolutely still needed.

I have attached a patch for same, please review

Next step would be to get a test case to show the issue.

At the very least I can say that patch #24 does apply and does fix the issue. Tested on Drupal 9.5.4

Merge request !13162 is a straightforward reroll— exactly what is in daniel korte's patch initial patch in #2 got into core through changes made out of this issue (without tests?!), so technically the problem stated in the title of this issue is now fixed in Drupal core. My re-roll of a merge request therefore only preserves jwilson's proposal in #12, implemented in gauravvvv's patch in #24, to not blow away attributes that were added by a possibly earlier preprocess.

I agree this is how it should work, but am not up for a test, let alone making this consistent everywhere, and i know there are rumblings about somehow getting rid of hook_preprocess entirely, however that would work, so i leave this humble re-roll and update as my offering.

Thanks for re-rolling!

Tagging for issue summary as the standard template appears to be missing.

Based on the title seems like something that tests should cover so am leaving that tag.

Thanks!