Seven's entity-add-list template omits link attributes

I think this is ready, but I'm the one who suggested that exact fix, so I shouldn't be the one to RTBC this.

This is blocking #2831274: Bring Media entity module to core as Media module.

Looks solid to me. I've been a bit absent from the queues, so create_attribute is new to me. I looked at the documentation, though, and this makes sense. Essentially passing the options.attributes from the add_link and manually placing them on the a element. Seems reasonable.

RTBC + 1

The template receives the link object directly and unpacks it so \Drupal\Core\Render\Element\Link::preRenderLink() is not used in this case.
For now I think it's RTBC.

When checking this, I saw the node add list and block content add list pages look the same but are built and rendered differently. It's currently not possible to add options for those links (the preprocessors are not providing options). It would be nice to make the developer/themer experience for this similar, but that's probably best discussed in a follow up.

At the very least this needs tests but also the divergence between node-add-list and this and also this and the other themes is a concern.

Patch looks good. Even a FAIL patch to prove it. Nice work!

Committed and pushed f01ac2d to 8.3.x and f5c7961 to 8.2.x. Thanks!

I've committed this to 8.2.x because seven is internal and the default entity-add-list.html.twig all just print bundle.add_link so will have the attributes.

Hm, I had a test fail on this in https://www.drupal.org/pift-ci-job/557210. Not reproducible.

Confirm the test fail related to this commit:

https://www.drupal.org/pift-ci-job/557855

Can't figure out where changes on removing test site directory might intersect with displaying link on a particular theme.

Yep this introduced a random fail :(

#2836237-11: Views with a different query plugin created via the UI do not have the correct query plugin ID in the view config and https://www.drupal.org/pift-ci-job/557961

I guess the random ness in the test - it'd be good to work out why.

I ran this test 150 times on my local machine and got zero test fails. Anyone an idea what to do here?

Looks something in core omits to initialize attributes for url somehow

I can't imagine this is causing a random fail. I wonder if the random fail was merely being exhibited through this change, but the root cause was elsewhere?

#29 makes me believe that even more.

#13 still applies cleanly. Reuploading.

There was definitely a random fail introduced by this issue in December. Being dismissive and claiming there was no random fail introduced by the patch is not helpful, because there was, and it started failing immediately after commit: https://www.drupal.org/pift-ci-job/557210. Whether or not the underlying cause was elsewhere, we can't have a test that randomly fails in core, because that's introducing a critical regression.

In order to commit it, we need to prove the random fail is no longer present, and in order to do that, we would need to locate the cause. Research into the fail needs to be documented on the issue.

Thanks everyone!

See #21, #22, #24, and #30.

Thanks @chr.fritsch for running the test locally. That's a good first step. I recommend also running it more times locally, like 1000. (150 runs only has a ~3 in 4 chance of surfacing a 1% fail.) This can be done on the commit hash for the original commit.

The next step is to see if it's something that fails on the bot but not locally, which is often the case. We can create a test-only patch that runs only this test several hundred times, in a test issue, adn then trigger multiple runs of that to get a baseline for the fail rate now.

To get a baseline fail rate on the bot for when this was committed, we could maybe use 8.2.x to test it (since it's unfeasible to post a patch that reverts the entirety of 7 months of code) or even 8.1.x, but let's start with those other steps.

Patch in #31 still applies cleanly on 8.5.x and tests are green locally:

php core/scripts/run-tests.sh --verbose  --url http://d8/ --class "Drupal\Tests\system\Functional\SevenBundleAddPageLinkAttributesTest"

Drupal test run
---------------

Tests to be run:
  - Drupal\Tests\system\Functional\SevenBundleAddPageLinkAttributesTest

Test run started:
  Sunday, October 1, 2017 - 04:36

Test summary
------------

Drupal\Tests\system\Functional\SevenBundleAddPageLinkAttribu   1 passes

Test run duration: 51 sec

Detailed test results
---------------------


---- Drupal\Tests\system\Functional\SevenBundleAddPageLinkAttributesTest ----


Status    Group      Filename          Line Function
--------------------------------------------------------------------------------
Pass      Other      SevenBundleAddPag   28 Drupal\Tests\system\Functional\Seve

I noticed that on default add bundle pages, where there are no extra attributes (the Url object options attribut is empty array, so options.attributes parameter is a NULL) there is an error like this

TypeError: Argument 1 passed to Drupal\Core\Template\TwigExtension::createAttribute() must be of the type array, null given

It means, that we cannot pass NULL to the createAttribute() method. It should be an array, even empty. I suggest add simple `Null coalescing operator` so we will pass in create_attrbute function eather initialized options attributes array from Url object options, or an empty array if the mentioned value is a NULL.

Tested #46 on 9.1
RTBC + 1

The Seven theme has been removed from Drupal 10 core. I confirmed that this issue only affects Seven and no other themes included with Drupal core, so I am moving this to the contributed Seven project.