Decouple image (URI) shape matching from specific image file types/extensions

The intent has been from the start to avoid adding "Drupalism" to SDC's use of JSON Schema.

That's why we started using /schema.json and defined $ref: json-schema-definitions://experience_builder.module/image-uri.

However, given that ✨ [PP-1] Make link widget autocomplete work (for uri and uri-reference props) Postponed is proposing to add a variation for format: uri named format: uri+entity_autocomplete (which required a core change: 🐛 ComponentValidator ignores the set validator and creates a new one Active ) 

So, an alternative HERE could be that we start using custom formats (for type: string). Which is exactly what @lauriii expressed he'd strongly prefer over at #3462705-47: [META] Missing features in SDC needed for XB → . (Note that JSON Schema does support custom format attributes: https://json-schema.org/draft/2020-12/json-schema-validation#section-7 + https://json-schema.org/draft/2020-12/json-schema-validation#name-custom...)

Or better yet: use contentMediaType.

So then we'd go from:

    "image-uri": {
      "title": "Image URL",
      "type": "string",
      "format": "uri-reference",
      "pattern": "^(/|https?://)?.*\\.(png|gif|jpg|jpeg|webp)(\\?.*)?(#.*)?$"
    },

to:

    "image-uri": {
      "title": "Image URL",
      "type": "string",
      "format": "uri-reference",
      "contentMediaType": "image/*"
    },

… which is not quite exactly how contentMediaType is meant to be used (that's really describing the contents of the string itself), but is … close enough? Something along these lines has been proposed as resourceMediaType.

Examples of actually defining new JSON Schema formats seem impossible to find.

This would then also allow us to apply a similar improvement to "video URLs", which is being added at ✨ Define JSON Schema $refs for linking/embedding videos and linking documents Active .

Related but different: 📌 Decouple image shape matching from the `image` MediaType Active .

Bumping to priority. Because without this, we'll need to keep doing awkward dances/updates and we'll never match all image fields.

IOW: without this, we won't be able to match any image fields or image media on existing Drupal sites that do not allow AVIF uploads. That's bad! XB users will be (rightfully!) baffled and frustrated that they can't use their years worth of uploaded images.

Same is true for video, which is currently limited to just MP4.

The pivot from uri-reference to uri should be uneventful, because as described in https://github.com/json-schema/json-schema/issues/81, URI references are a subset of URLs.

But in practice, this does not seem to be the case, due to the utterly broken JSON Schema validation of URIs in justinrainbow/json-schema. Hopefully #3540470-14: Support for latest 6.x version of justinrainbow/json-schema package → brings relief.

For now, sticking to uri-reference to reign in scope.

wim leers → closed merge request !1464

Looks like #7 is turning out to become a reality, but for a different reason 🙃

The problem appears to be that pattern: ^(/|https?://)? is not considered valid in 5.3 (which CI is testing with), but is considered valid in 6.4. 🫠 Which I'm developing against locally, and I don't really have a choice, because I'm on a non-ancient version of Composer, which forces this:

$ composer require justinrainbow/json-schema:^5.3
justinrainbow/json-schema is currently present in the require-dev key and you ran the command without the --dev flag, which will move it to the require key.
Do you want to move this requirement? [no]? yes
./composer.json has been updated
Running composer update justinrainbow/json-schema
> Drupal\Composer\Composer::ensureComposerVersion
Loading composer repositories with package information
Updating dependencies
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Root composer.json requires justinrainbow/json-schema ^5.3, found justinrainbow/json-schema[5.3.0, 5.x-dev] but these were not loaded, likely because it conflicts with another require.
  Problem 2
    - composer/composer is locked to version 2.8.9 and an update of this package was not requested.
    - composer/composer 2.8.9 requires justinrainbow/json-schema ^6.3.1 -> found justinrainbow/json-schema[dev-master, 6.3.1, ..., 6.x-dev (alias of dev-master)] but it conflicts with your root composer.json require (^5.3).

Use the option --with-all-dependencies (-W) to allow upgrades, downgrades and removals for packages currently locked to specific versions.

Installation failed, reverting ./composer.json and ./composer.lock to their original content.

The 5.3 regex validation logic is apparently this:

    protected function validateRegex($regex)
    {
        return false !== @preg_match('/' . $regex . '/u', '');
    }

— https://github.com/jsonrainbow/json-schema/blob/5.3.0/src/JsonSchema/Con...

Which indeed fails: https://3v4l.org/hPCA3.

In 6.3.1 and later it is very different: https://github.com/jsonrainbow/json-schema/blob/6.3.1/src/JsonSchema/Con...

LOLOL reading 📌 Allow 6.x version of justinrainbow/json-schema Active points to Composer 2.8.6 vs 2.8.8, while here 2.8.10 is used. https://github.com/composer/composer/releases/tag/2.8.10 points to https://github.com/composer/composer/pull/12376, which is … very much a similar problem 😅 (It touches upon pattern being quite brittle, and literally does what we did in 🐛 Img prop constraints require extension to be lower case Active !)

The justinrainbow/json-schema version bump didn't work, root cause was #3538439-13: Running tests locally is difficult to match with the CI → . Being fixed at #3540470-16: Support for latest 6.x version of justinrainbow/json-schema package → .

Will continue after the weekend.

This MR was getting unwieldy due to 2 bits we overlooked (well, I failed to spot as a reviewer) in 📌 Create an Image SDC that can be included by other SDCs Active :

1. 🐛 `noUi: true` SDCs do still show up in the UI Active
2. 🐛 `card-with-stream-wrapper-image` test SDC generates an invalid `` Active

Merged in upstream now that 🐛 `noUi: true` SDCs do still show up in the UI Active is in 👍

New blocker: 🐛 Resolved image file URLs invalid in kernel tests Active — will make this MR far less overwhelming. Also: 🐛 `card-with-stream-wrapper-image` test SDC generates an invalid `` Active is green — awaiting @justafish's approval.

🐛 Resolved image file URLs invalid in kernel tests Active is in, bringing that into the branch reduces it from 39 to 27 files.

🐛 `card-with-stream-wrapper-image` test SDC generates an invalid `` Active will bring it down more 👍

#7 was fixed in 🐛 Resolved image file URLs invalid in kernel tests Active .

🐛 `card-with-stream-wrapper-image` test SDC generates an invalid `` Active is also in — yay! Down to 25 files changed now.

I won't be able to finish this today. To make card-with-stream-wrapper-image work correctly as well requires further fixes.

Also: the whole pattern (aka Regex) based matching seems brittle. It's better in this MR than it is in HEAD, and pretty proven thanks to the new SchemaJsonPatternsTest, but it's still too brittle.

I propose to stop using pattern altogether and instead introduce x-allowed-schemes, similar to how we already introduced x-required-variables and x-formatting-context. Will refactor towards that on Monday. 👍

Green!

This removes 7 @todos and adds 3.

Literally the last failure is

    // Valid stream wrapper, but non-existent file.
    await xBEditor.exitPreview();
    await xBEditor.openLayersPanel();
    await xBEditor.openComponent('Card with stream wrapper image');
    await xBEditor.editComponentProp('src', 'public://balloons2.png');
    await xBEditor.preview();
    const cardStreamWrapperInvalidSrc = previewIframe.locator(
      '.card--with-stream-wrapper-image img.card--image',
    );
    await cardStreamWrapper.scrollIntoViewIfNeeded();
    expect(await cardStreamWrapperInvalidSrc.getAttribute('src')).toContain(
      // The stream wrapper URI must be rewritten to an actually resolvable URL.
      // @see tests/modules/xb_test_sdc/components/card-with-stream-wrapper-image/card-with-stream-wrapper-image.twig
      'files/balloons2.png',
    );

… which is testing with an INVALID stream wrapper URI entered into a input[type=text], which the XB UX is intended to never show/allow, and indeed is no longer the case: it now shows the image field type's widget!

So: removing that bit of test coverage. Will try to refactor it in the morning to make it pick an image from the media library and hence prove it works end-to-end. Will then merge this MR and address #22 in a subsequent MR.

Change record updated: https://www.drupal.org/node/3542579 →

So: removing that bit of test coverage. Will try to refactor it in the morning to make it pick an image from the media library and hence prove it works end-to-end.

Done! As soon as this is green, will merge this MR and address #22 in a subsequent MR.

Had to add explicit waits because CI is so slow, but it's passing now! 🥳

That's in! Now back to for #22.

wim leers → credited effulgentsia → .

@effulgentsia just +1'd #22 in a meeting 😊👍

Will land this tomorrow AM to reduce the change rate for #3519247 now that it's unblocked again per #3519247-23: Acquia DAM and Canvas integration → .