Allow configure image style base url

Added drupal installation prefixes support.

Some concerns/questions I have on an initial review:

1. We really should be targeting 4.x first so we can avoid a repeat of what occurred with 7.x->8.x, where we even now still find features that were never ported.
2. How would a site do this if they were not running the s3fs module? Does core already offer this feature somewhere and we didn't implement it?
3. This needs an update to the test files.
4. We are currently moving away from using settings.php as 4.x will be exclusively using Config Entities in order to support multiple clients, buckets, and streamWrappers. I would prefer we not add on a new settings.php option that can not be ported into the new system.

5. +++ b/src/Form/SettingsForm.php
   @@ -408,6 +408,23 @@ class SettingsForm extends ConfigFormBase {
   +          ':input[id=edit-use-s3-for-public]' => ['checked' => TRUE],
   

   Wouldn't this setting also impact s3:// ? If so shouldn't it always be visible?

6. +++ b/src/PathProcessor/S3fsPathProcessorImageStyles.php
   @@ -58,10 +64,10 @@ class S3fsPathProcessorImageStyles implements InboundPathProcessorInterface {
   +    return str_contains($path, static::IMAGE_STYLE_PATH_PREFIX);
   

   This appears to be too wide. We don't want to match a string that needs to be at the beginning when it isn't. Odd False Positive issues could occur.

   I can't see on a cursory review why this would need to change?

I will note I moved this project into GitLabCI a few months ago in order to allow us to slowly improve our testing regiment (we now have static analysis and code coverage enabled, and I'm looking at us adding Mutation testing in the future) as such to run tests we need to use MR's going forward.

Hello cmlara,

Thanks for reviewing!

Reading your review, maybe this feature it should be at core level and not in a specific module like s3f, because It's interesting that works with or without s3fs module.

If you see it, we can decline the issue and I could create a new one in drupal core.

As we know that the speed in core is slower than contrib modules, if in the future somebody or the maintainers decide that the feature is interesting, we can do directly to config entities in 4.x branch and cover with tests.

What do you think?

Greetings.

My biggest thought is, how niche a feature is this? Is this something a sizeable percentage of the install count will want or only a few? The goal being to try and avoid adding new code that will end up used infrequently.

One advantage of 4.x's architecture is that everything will be plugin based, so we don't have to be as aggressive adding features, sites will be able to add a new plugin if they need a feature that is unique to them.

I do wonder if perhaps the CDN module should be providing this feature? This sounds like something that belongs to them instead of Core?

I really don't know how niche feature is, in our case we prefer all statics extensions to process better in differents layers.

4.x's architecture looks very good, congrats! I want to test it soon!

I have the same doubt, maby CDN module is better or core. For instance in core we can set file_public_base_url, for me its something related but maybe it's a feature that it's interesting for CDN module, it's true that sometimes CDN module wasn't enough for some projects and we had to do a custom CDN integration, but I can propose in both projects, related both issues and hope that they with a big picture decide the better choice if they think that this feature is interesting.