Add support of "preload" attribute into FileMediaFormatterBase

@alexpott I'd love to get this in, I've added @pclaitte changes onto an MR and the following changes:

- Widget is now radios as suggested
- Added short descriptions for the 3 options.
- Added a post update with a config updater

I think the update needs a test but I'd appreciate some guidance on where to find something similar (this has been my first encounter with the config entity updater)

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request → as a guide.

This seems like it's going to need an upgrade path for existing settings.

Patch looks good to me, already pretty polished...

What I'm wondering about is: how "safe" is to force a default preload attribute value on sites that didn't have any?

I mean, if I had a page full of videos with no 'preload' attribute, maybe just an image placeholder, and suddenly I get that preload=metadata default.... will browsers just start preloading when they didn't before?

The point is: does it make sense to force a default value? or to force any value at all? Should we skip the update script until the user actually edits the formatter?

For background, I've found this pretty interesting post about how browsers behave, but note it is 10 years old, couldn't find and updated one... https://www.stevesouders.com/blog/2013/04/12/html5-video-preload/

See the upgrade path but it needs test coverage. Could be something simple as

Before this value was null
Run updates
This value is now false

Didn’t test the MR

@smustgrave please see #35 regarding update path.

Yes the update hook is there but there is no test coverage of it.

See BlockContentUpdateTest for example

@smustgrave the question in #35 refers to whether we should have the update hook in the first place.

So if I do a config export.
Go into a field or display using this formatterBase.
If I make 0 changes but save the field and do drush export again and it's showing this new setting then yes it will need an update path.

If no then it will not.

The update hook should set to disabled value though.

That help some?

@smustgrave "Should we skip the update script until the user actually edits the formatter?" That's exactly what #35 is explaining.

It's ok to assume changes can happen when I save the formatter, but NOT when I just upgrade my site. That could have unexpected consequences, and that's what #35 is asking for feedback from @alexpott.

Actually the opposite if I save a formatter without making a change and I get changes in my config now that should not be expected.

Same for how we handle other field changes but will let alexpott respond.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

I only scanned the MR quickly, but I couldn't see the a presave hook to update shipped config (i.e. default config in modules), that should happen as well as the update function.

We need to update the configuration to something in both post update and presave, so that sites don't get unexpected config changes in runtime - better if they get them during a core update which is when they'll be looking for them.

So.. I think it comes down to what a non-intrusive update looks like - i.e. should it be 'metadata' or 'none', or do a need an additional option for 'unspecified' to preserve current behaviour and update sites to that?

Didn't look around for docs on browser behaviour yet so not sure what the answer is, but definitely we should update to something here.

Rebased 10.1.x into MR; providing patch for 9.5 in case anyone needs it.
Leaving in NW as per #45.

📌 Add image preload option to help boost actual and perceived performance Needs work is doing some similar things for images. It also has some of the bits around update hooks that are being asked about in #40 and below. For already committed code, look at what ResponsiveImageConfigUpdater is doing.

Can we update the IS to differentiate between what _this_ issue is doing vs #3309016?

mrshowerman → changed the visibility of the branch 11.x to hidden.

Created MR against 11.x. Adding patch for 10.3.x.

Has this not been implemented? If so, how do I find it? I do not want to apply a patch for something that won't show up in production. But I am sorry for wasting anyone's time if I am missing something...

I have no preload options at /admin/structure/media/manage/audio/display

Not being able to prevent preloading of media in D10 is a deal-breaker for me, I cannot use Drupal media. I create sites with audio library pages using views, and when a page of multiple audio files is displayed, sometimes dozens, I cannot have them all downloading.

For this reason, I am compelled to retain the method I used in Drupal 7, which was to create fields with preconfigured HTML5 code using tokens. Doing that I need a file field and two text fields for every audio file (the easiest way without making a custom module, as I am not a developer).