fix : il ne faut pas contraindre la taille des SVG #4893

Merged
cerdic merged 2 commits from RealET/medias:dimensions_svg into master 1 month ago
RealET commented 2 months ago

(et plus généralement des images vectorielles)

Testable avec ceci dans config/mes_options.php :

if (!defined('_LOGO_MIN_WIDTH'))
	define('_LOGO_MIN_WIDTH', 1920);
if (!defined('_LOGO_MAX_WIDTH'))
	define('_LOGO_MAX_WIDTH', 3840);

et un svg de moins de 1920 de viewport (n'importe quelle icone de plugin fait l'affaire)

(et plus généralement des images vectorielles) Testable avec ceci dans config/mes_options.php : ``` if (!defined('_LOGO_MIN_WIDTH')) define('_LOGO_MIN_WIDTH', 1920); if (!defined('_LOGO_MAX_WIDTH')) define('_LOGO_MAX_WIDTH', 3840); ``` et un svg de moins de 1920 de viewport (n'importe quelle icone de plugin fait l'affaire)
RealET added 1 commit 2 months ago
abefe9ddaa fix : il ne faut pas contraindre la taille des SVG
Owner

Ça se tient :) Version testée ?

Ça se tient :) Version testée ?
b_b added the
bug
label 2 months ago
b_b approved these changes 2 months ago
Poster

Ça se tient :) Version testée ?

Testé en SPIP 4.2 git du jour.

> Ça se tient :) Version testée ? Testé en SPIP 4.2 git du jour.
Poster

Je me demande s'il ne vaudrait pas mieux un define pour 'svg' avec la possibilité que ce soit plusieurs formats (je sais qu'il n'y a que svg actuellement, mais ça pourrait changer dans l'avenir…).

Un avis ?

Je me demande s'il ne vaudrait pas mieux un define pour 'svg' avec la possibilité que ce soit plusieurs formats (je sais qu'il n'y a que svg actuellement, mais ça pourrait changer dans l'avenir…). Un avis ?
Owner

Je ne sais pas tiens. Potentiellement un SVG peut embarquer un .png en base64.
Ce qui est très con quand ces le cas.

Je ne sais pas tiens. Potentiellement un SVG peut embarquer un .png en base64. Ce qui est très con quand ces le cas.
cy.altern approved these changes 1 month ago
Owner

le patch me parait très incorrect : ce n'est pas parce qu'on ne veut pas que le SVG soit soumis aux contraintes de tailles en px que pour autant il doit passer dans les tests de poids de _DOC_MAX_SIZE car jusqu'à preuve du contraire c'est toujours une image, et il doit donc passer au controle via _IMG_MAX_SIZE ou _LOGO_MAX_SIZE

le patch me parait très incorrect : ce n'est pas parce qu'on ne veut pas que le SVG soit soumis aux contraintes de tailles en px que pour autant il doit passer dans les tests de poids de `_DOC_MAX_SIZE` car jusqu'à preuve du contraire c'est toujours une image, et il doit donc passer au controle via `_IMG_MAX_SIZE` ou `_LOGO_MAX_SIZE`
cerdic requested changes 1 month ago
cerdic left a comment
Owner

un svg est toujours une image et donc on doit rester dans les tests qui concernent les images

un svg est toujours une image et donc on doit rester dans les tests qui concernent les images
b_b requested changes 1 month ago
b_b left a comment
Owner

Ha mais oui, j'ai complètemement foiré ma relecture, le test est mal placé puisqu'il échape le test de poids et non le test de dimension.

Ha mais oui, j'ai complètemement foiré ma relecture, le test est mal placé puisqu'il échape le test de poids et non le test de dimension.
RealET added 1 commit 1 month ago
RealET requested review from cerdic 1 month ago
RealET requested review from b_b 1 month ago
RealET requested review from cy.altern 1 month ago
Poster

Je ne sais pas tiens. Potentiellement un SVG peut embarquer un .png en base64.
Ce qui est très con quand ces le cas.

Mais le comportement de cette image en base64 reste vectoriel, avec taille d'affichage choisie par le svg et les styles CSS, non ?

Autrement dit, c'est juste une texture qui sera plus ou moins pixelisée selon le rendu final, non ?

> Je ne sais pas tiens. Potentiellement un SVG peut embarquer un .png en base64. > Ce qui est très con quand ces le cas. Mais le comportement de cette image en base64 reste vectoriel, avec taille d'affichage choisie par le svg et les styles CSS, non ? Autrement dit, c'est juste une texture qui sera plus ou moins pixelisée selon le rendu final, non ?
cerdic merged commit dc1fc5e684 into master 1 month ago

Reviewers

cerdic was requested for review 1 month ago
b_b was requested for review 1 month ago
cy.altern was requested for review 1 month ago
The pull request has been merged as dc1fc5e684.
Sign in to join this conversation.
No Milestone
No Assignees
5 Participants
Notifications
Due Date

No due date set.

Dependencies

This pull request currently doesn't have any dependencies.

Loading…
There is no content yet.