Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.1] Proper support for avif/webp images #43295

Merged
merged 8 commits into from
May 14, 2024
Merged

Conversation

dgrammatiko
Copy link
Contributor

@dgrammatiko dgrammatiko commented Apr 18, 2024

Pull Request for Issue # .

Summary of Changes

  • add missing avif support in the Image class
  • The resize plugin cannot create an image from string for AVIF files (someone needs to open an issue to php-src for this)
  • both webp and avif cannot be supported for editing because browsers are inconsistent atm on the canvas.toDataURL()
    spec: toDataURL

Testing Instructions

  • Either run npm I or use the package from the PR

  • extract and upload the files Archive.zip

  • Delete/rename the files

  • Delete the files. Edit the Media Action plugin resize with some values (ie 800 and 500) and check that the images on upload are getting resized.

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.1-dev labels Apr 18, 2024
@dgrammatiko dgrammatiko changed the title avif/webp [5.1] Proper support for avif/webp images Apr 18, 2024
@dgrammatiko dgrammatiko marked this pull request as ready for review April 18, 2024 20:06
Co-authored-by: Quy <quy@nomonkeybiz.com>
@Quy
Copy link
Contributor

Quy commented Apr 19, 2024

I have tested this item ✅ successfully on 6f7b7cb


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43295.

1 similar comment
@viocassel
Copy link
Contributor

I have tested this item ✅ successfully on 6f7b7cb


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43295.

@Quy
Copy link
Contributor

Quy commented Apr 19, 2024

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43295.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Apr 19, 2024
@dgrammatiko
Copy link
Contributor Author

Can we remove the RTC. Turns out that PHP docs were wrong php/php-src#14010

Please test again with:

  • Delete the files. Edit the Media Action plugin resize with some values (ie 800 and 500) and check that the images on upload are getting resized.

@viocassel
Copy link
Contributor

I have tested this item ✅ successfully on 84874f6

👍


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43295.

@richard67
Copy link
Member

Back to pending. @Quy Could you test again? Thanks in advance.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43295.

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Apr 20, 2024
@brianteeman
Copy link
Contributor

It was commented elsewhere that this PR would also resolve an issue with files named JPG as opposed to jpg.

Sorry to say this pr makes no difference

@Quy
Copy link
Contributor

Quy commented Apr 20, 2024

I have tested this item ✅ successfully on 84874f6


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43295.

@Quy Quy added the RTC This Pull Request is Ready To Commit label Apr 20, 2024
@Quy
Copy link
Contributor

Quy commented Apr 20, 2024

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43295.

@HLeithner HLeithner changed the base branch from 5.1-dev to 5.2-dev April 24, 2024 09:06
@HLeithner
Copy link
Member

This pull request has been automatically rebased to 5.2-dev.

@dgrammatiko
Copy link
Contributor Author

@HLeithner why did you moved this one to 5.2? This is a bug fix nothing new here...

@HLeithner
Copy link
Member

HLeithner commented Apr 24, 2024

I only had quick look and just classified it as feature since nobody else looked at it and added a label. If I'm wrong no problem can be changed back as long as nobody update the branch.

@dgrammatiko
Copy link
Contributor Author

dgrammatiko commented Apr 24, 2024

AVIF support was added with #41381 but George missed some parts. This PR just adds them, it doesn't have to wait for 6 months

Edit: actually it fixes the issue reported in the introductory PR: #41381 (comment) where you also responded: #41381 (comment)

@HLeithner HLeithner added bug and removed Feature labels Apr 24, 2024
@HLeithner HLeithner changed the base branch from 5.2-dev to 5.1-dev April 24, 2024 10:02
@HLeithner
Copy link
Member

switched back to 5.1 thanks

@bembelimen bembelimen merged commit 8b79de7 into joomla:5.1-dev May 14, 2024
3 of 4 checks passed
@bembelimen
Copy link
Contributor

Thx

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 14, 2024
@bembelimen bembelimen added this to the Joomla! 5.1.1 milestone May 14, 2024
@dgrammatiko dgrammatiko deleted the image branch May 14, 2024 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.1-dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants