🇺🇦Ukraine @Taran2L

so, it does not support HEIF for processing, only for getting the image dimensions - and the code to do that is the same as avif:

		case IMAGE_FILETYPE_AVIF:
			result = php_handle_avif(stream);
			break;
		case IMAGE_FILETYPE_HEIF:
			if (!php_stream_rewind(stream)) {
				result = php_handle_avif(stream);
			}
			break;

and the reason for that is (I believe): both standards are basically use the same container format ... technically AVIF is an AV1 encoded image in HEIF container, while HEIC is an HEVC encoded image in HEIF container. Very confusing actually :D

The problem is that HEIF image can ... pretend not not be HEIF, and ... be an AVIF

See https://github.com/nokiatech/heif/issues/74 for details

Our test image is like example C034.heic:

0x0000001C  // 28-bytes: size of 'ftyp' box.
'ftyp'      // boxtype
'mif1'      // major_brand
0x00000000  // minor_version
'mif1'      // compatible_brands[0]
'avid'      // compatible_brands[1]
'miaf'      // compatible_brands[2]

so it has mif1 and avif, but no hei* ... but PHP detects mif1 and decide that tis is an heir image which it's not true

I think PHP has broken AVIF detection by introducing HEIF/HEIC support here https://github.com/php/php-src/commit/dfac2da7fba930570fcda99205cfcf53db...

This affects multilingual sites only with a domain per language setup (and not a path prefix)

This has not been fixed!

taran2l → changed the visibility of the branch 3540531-test-on-php-85 to hidden.

So, the GD issue with rotate was indeed related to the default interpolation method change (should core now keep the BC and set it explicitly?)

The AVIF issues is mystery, as it fails on loading the image rather than processing. Is AVIF part of the docker image?

And due to new deprecations (PDO constants) most of the tests are red

taran2l → changed the visibility of the branch 3540531-fix-toolkitgdtest-for to active.

taran2l → changed the visibility of the branch 3540531-fix-toolkitgdtest-for to hidden.

So, it seems that the default interpolation algorithm has been changed, see https://github.com/php/php-src/pull/17375/files#diff-a6d1ba8e66261eacec5...

That probably means that we need to adjust test to that

Seems like rotation is affected either by our code or by how PHP8.5 handles it, as all rotate to 5 degrees tests are broken (file format is not relevant, it happens for all of them)

See https://git.drupalcode.org/issue/drupal-3540531/-/jobs/6225263#L2188

The other part is AVIF, I assume Docker image does not have AVIF support at all (or it's broken in PHP itself)

So, my idea was to recreate the file itself (maybe it has encoding issues), but that didn't work ...

taran2l → made their first commit to this issue’s fork.

cmlara → credited taran2l → .

cmlara → credited taran2l → .

hi @chizh273, there are more changes in the Drupal.nl private repo commit related to this issue, please transfer those over, see https://github.com/lembergsolutions/drupal.nl/commit/b1f22377d7718d98853...

taran2l → made their first commit to this issue’s fork.

taran2l → made their first commit to this issue’s fork.

taran2l → created an issue.

taran2l → made their first commit to this issue’s fork.

that one looks good to me

taran2l → created an issue.

addressed feedback

taran2l → made their first commit to this issue’s fork.

Per great suggestion by @danielveza, I have converted the test to Functional

Hope this can go in finally

taran2l → created an issue.

taran2l → created an issue.

taran2l → created an issue. See original summary → .

@cmlara thanks for acting on this so fast!

cmlara → credited taran2l → .

cmlara → credited taran2l → .

As I mentioned I've went ahead and changed scope a bit.

• I've simplified version constraints
• Enabled testing with previous major version (so 10.x is being tested)
• Add support for PHPStan 2.0 (which generates a lot of issues, probably a separate issue needed to resolve or ignore those)
• 11.2 support should can be handled separately, maybe here 📌 Copy core 11.1.x ExtensionMimeTypeGuesser to s3fs Active

PS I would suggest dropping support of D8 and D9 altogether, as well as PHP7 .. it's not being tested anyway, so is it still working ?

Limiting the scope to 10.5 per @cmlara suggestion

kraut → credited taran2l → .

taran2l → made their first commit to this issue’s fork.

Implemented @luke.leber suggestion, please review. This should be ready to go in now

@catch, in #80 @tim.plunkett has self-assigned this issue and no progress for over a year ... is anyone has a better idea on how to overcome this issue without dependency on the route system?

Per @smustgrave suggestion I've made messages translatable, but this causes unexpected fatal errors in a few tests (I assume sometimes string_translation service is not ready yet/not available, but redirect already happens)

So, I've reverted those changes, so messages are as they are (i.e. not translatable)

However, I've one concern: UrlHelper is a simple class that does not depend on the container at all, and we are adding hook invocation to it.

A few tests are still failing, will take a look later

Addressed feedback by @smustgrave

taran2l → made their first commit to this issue’s fork.

Converted patch from #3098245-8: Clone nested inline blocks when translating → into MR + updated it after ✨ Fails on translated inline blocks Closed: duplicate got merged.

taran2l → made their first commit to this issue’s fork.

taran2l → made their first commit to this issue’s fork.

Went ahead and fixed this .... still supporting 9.5, it's up to you if you want to go like that

taran2l → made their first commit to this issue’s fork.

thanks for the fast turnaround!

Does not work actually

Blocked by 🐛 Error uploading svg from the media tab Active

taran2l → changed the visibility of the branch 3482121-error-uploading-svg to hidden.

hi @jwilson3

this pipeline shows that it works now with D11 -> https://git.drupalcode.org/issue/svg_image_field-3482121/-/pipelines/403938

I'm removing gitlab CI staff from this one, as there is another one for that already 📌 Make CI green again Active

taran2l → made their first commit to this issue’s fork.

taran2l → created an issue.

taran2l → made their first commit to this issue’s fork.

taran2l → made their first commit to this issue’s fork.

Should be OK now - no PHP warnings + all validators are preserved

Uh ... it kills the built-in validations now ...

taran2l → created an issue.

hi @dblanken @seanb

I've added FieldableEntityInterface back, the original change is doing a good refactoring, but it out of scope for this issue imo and makes it harder to understand and review (plus it's harder to combine patches)

the conversion to OOP hooks broke this MR, corresponding code should be moved to the class files, I can take a look later this week

taran2l → created an issue.

taran2l → made their first commit to this issue’s fork.

I've removed all refactoring to make everything a little bit easier to review and added a BC layer for the service constructor arguments change

taran2l → changed the visibility of the branch 8.x-1.x to hidden.

With a post update hook it works as expected. Please review

No, sorry, it does not work like that

taran2l → made their first commit to this issue’s fork.

Green again, I think this can go in ?

thanks @voleger