Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Image Bug / Compression Bug! #5268

Closed
pixelmount opened this Issue · 18 comments

5 participants

@pixelmount

Hallo,
ich habe einen schon wahrscheinlich länger existierenden Bug gefunden (zumindest konnte ich dieses Problem schon seit Version 2.11.7 beobachten). Ich habe den Fehler auf 2 verschiedenen Servern und mit frisch aufgesetztem CMS reproduzieren können, die Bildeinstellungen bei "Einstellungen" wurden dabei auf Maximalwerte von 2000-3000 getestet (Frontendbildbreite sowie GD-Bildverarbeitungsbildbreite und ebenfalls CHMOD Rechte 777 für temporäre Verzeichnisse sowie das Bild selbst wurden gesetzt, sowie mit und ohne aktivierter .htaccess getestet). Bei Bildern die eine Größe von mehr als 1200px Breite haben wird keine Bild-Komprimierung / Resize Funktion vom CMS verwendet. Einfaches Beispiel das ich im CMS getestet habe:

  1. Im CMS bei den Artikeln einfach ein neues Bild einfügen das z.B. 1440px Breite hat und bei "Bildbreite" beispielsweise 1300 eingeben.
  2. Im Frontend mit einem Dev-Inspect Tool (Firebug o.ä.) nachschauen, und man sieht dass das CMS kein neues Bild mit einer Breite von 1300px im temporären Ordner generiert hat, stattdessen benutzt er einfach nur das Original mit 1440px Breite.

Sobald man die Bildbreite aber auf "1200" oder niedriger stellt setzt die Komprimierung vom CMS korrekt an. Ich habe euch hierfür einen Testzugang eingerichtet, die Rubrik "Einstellungen" habe ich nur "lesbar" gemacht, die Einstellungen sind aus Sicherheitsgründen nicht bearbeitbar.

Das Frontendergebnis könnt ihr hier betrachten: http://pixelmount.com/contao2118/
Zum Backend geht es hier lang: http://pixelmount.com/contao2118/contao
Username: test
Password: test9999

Das Bild ist eine Eigenkreation und darf weder kopiert, weiterverwendet oder zu kommerziellen Zwecken benutzt werden.

Viele Grüße
S.P.

@pixelmount pixelmount closed this
@pixelmount pixelmount reopened this
@BugBuster1701

Kein Bug, Feature.
https://github.com/contao/core/blob/hotfix/2.11.9/system/libraries/Controller.php#L1009
Größere Bilder können durch GD zu Memory Problemen führen.
Stattdessen könnte man die MagickImages Erweiterung nehmen.

@pixelmount

Das sollte mehr als ein Downgrade bezeichnet werden und nicht als Feature, wobei dies auch noch Nebeneffekte hat. Woher soll der User des CMS überhaupt wissen dass bei bestimmten Bild-Attributen die Image-Datei nicht korrekt verarbeitet werden kann? (Es ist noch nicht mal ein Hinweis vorhanden), außerdem wären dann ja die Einstellungen "Maximale Frontend-Breite" und "Maximale GD-Bildbreite" absolut überflüssig wenn ohnehin der Wert "1200" verwendet wird. Wieso setzt man stattdessen nicht einfach den Wert "1200" als Default in "Maximale GD-Bildbreite"? Das hätte doch den gleichen Effekt und untergräbt nicht die Funktionalität des CMS, ganz geschweige von der Verwirrung des Users.

@tristanlins

@BugBuster1701 das ist definitiv ein Bug, das wurde nämlich mal umgebaut, eben wegen dem was @pixelmount sagt! Dafür gibt es die Einstellung "Maximale GD-Bildbreite" und "Maximale GD-Bildhöhe".

@BugBuster1701

OK, mein Fehler

if (!extension_loaded('gd') 
|| !$objFile->isGdImage 
|| $objFile->width > $GLOBALS['TL_CONFIG']['gdMaxImgWidth'] 
|| $objFile->height > $GLOBALS['TL_CONFIG']['gdMaxImgHeight'] 
|| (!$width && !$height) 
|| $width > 1200 
|| $height > 1200)
//dann nix tun

Heißt, wenn die Originalgröße > als Maximale GD Breite/Höhe ist
oder keine neue Breite und Höhe angegeben ist
oder die neue definierte Bild Breite oder Höhe > 1200 px ist.

D.h., die Änderung damals hat zwar größere Dateien akzeptiert die überhaupt zu betrachten, aber wenn die neue Größe 1200px übersteigt wird wieder nix gemacht. Ob das nun Absicht oder Bug ist, kann ich nicht beurteilen.

@BugBuster1701

Ihr wollt wohl sowas?

if (!extension_loaded('gd') 
|| !$objFile->isGdImage 
|| $objFile->width > $GLOBALS['TL_CONFIG']['gdMaxImgWidth'] 
|| $objFile->height > $GLOBALS['TL_CONFIG']['gdMaxImgHeight'] 
|| (!$width && !$height) 
|| $width > $GLOBALS['TL_CONFIG']['gdMaxImgWidth'] 
|| $height > $GLOBALS['TL_CONFIG']['gdMaxImgHeight'] )
//dann nix tun
@Babelfisch Babelfisch referenced this issue in 4t2/retina_image
Open

Limitierung bei Bildgrössen? #2

@pixelmount

@BugBuster1701 Danke für den Lösungsvorschlag, der letzte Code von dir sieht schon besser aus. Auch wenn ich mir noch nicht ganz sicher bin wieso es einmal $width und $objFile->width gibt, enthalten beide Variablen nicht den gleichen Wert?

Die Bedingung sollte grundsätzlich falls möglich nie einen statischen Wert (wie in dem Fall "1200") enthalten, das schränkt meines Erachtens nur die Flexibilität des Users ein. Begutachtet einer der Contao-Programmierer diesen Bug? Bisher scheint es so als fühle sich hierfür niemand zuständig da das Ticket noch niemandem zugewiesen worden ist.

@Babelfisch

@pixelmount: Wenn ich das richtig sehen, ist $width die Größe, auf die das Bild skaliert werden soll und $objFile->width die Größe der Datei selbst.

@pixelmount

@Babelfisch Macht Sinn xD Hätte ich auch drauf kommen können.

@tristanlins Ich will ja kein komplett neues Paket nur wegen einem einzigen Bug ;) Was genau unterscheidet dein Paket zum Normalen? In der Readme konnte ich nichts vorfinden das die Features auflistet :)

@Babelfisch

Nur noch mal zum Thema Bug oder nicht – also es ist natürlich auf jeden Fall ein Bug, da es ja keinen Sinn macht, einerseits per Einstellungen große Bilder zu erlauben, dann aber hart kodiert zu verbieten, diese Dateien größer als 1200 Pixel zu skalieren.

@BugBuster1701: Kannst du deine Änderung nicht gleich als Pull-Request oder so schicken? Da geht es vielleicht schneller und wird hoffentlich schon in den nächsten Version behoben.

@tristanlins

@pixelmount äh, da steht doch unter Modifications https://github.com/bit3/contao-core-infinitysoft#modifications drin?
Es gibt ja keine neuen Features, es ist ein Contao mit ein paar Modifikationen ;-)

@leofeyer
Owner

Welche Lösung wäre denn nun die bessere? $width > $GLOBALS['TL_CONFIG']['gdMaxImgWidth'] oder die Bedingung komplett entfernen so wie in Tristan's Patch?

@Babelfisch

Ich würde zu $width > $GLOBALS['TL_CONFIG']['gdMaxImgWidth'] tendieren. Bilder müssen nicht über die maximale GD-Bildbreite/-höhe skaliert werden und daher ist diese zusätzliche Überprüfung sinnvoll, um bspw. Fehleingaben und damit eine mögliche hohe Serverauslastung abzufangen.

@pixelmount

Ich stimme Babelfisch zu, ich tendiere ebenfalls zu $width > $GLOBALS['TL_CONFIG']['gdMaxImgWidth'];

Als blödes Beispiel, wenn das orig. Bild eine Breite von 600 hat und der User bei "Bildbreite" aus welchem Grund auch immer 9000 angibt sollte das nicht angenommen werden, daher muss das durch eine Bedingung limitiert werden (sonst wäre das z.B. eine Möglichkeit für einen "beabsichtigten" Overload des Servers aus welchen Gründen auch immer so etwas zu gebrauchen wäre ;) aber natürlich könnte es auch ebenso ein Tipfehler sein).

@leofeyer leofeyer referenced this issue from a commit
@leofeyer leofeyer Replaced the 1200 pixel limit when resizing images with the values de…
…fined in the system settings (see #5268)
825ed34
@leofeyer
Owner

Behoben in 825ed34.

@leofeyer leofeyer closed this
@pixelmount

Klasse, vielen dank für den kommenden Bugfix :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.