Jump to content

Welcome, Guest!

By registering with us, you'll be able to discuss, share and private message with other members of our community.

Traumflug

Administrators
  • Content Count

    1,300
  • Joined

  • Last visited

  • Days Won

    35

Posts posted by Traumflug


  1. P.S.: I'd also download a few images and open them in an image viewer to find out whether they're actually JPEGs (or PNGs with suffix .jpg). I've seen shop installations where properly named files were there, but none of them being actual images.


  2. Looking at this page, one image is missing:

    https://product.solutions.org.nz/remote-car-alarm-with-central-locking-and-engine-disable

    Digging a bit deeper, this is the image URL:

    https://product.solutions.org.nz/194-community-theme-default_cart_default/remote-car-alarm-with-central-locking-and-engine-disable.jpg

    This URL looks fine to me. Shop is apparently using theme community-theme-default, this theme has the old image type names (with _default), so that's what's expected. Also, some other images work, which makes a glitch in image finding unlikely. Broken code usually applies to all images, not just a few.

    I'd start looking at which images are actually on disk. This image should be in img/p/1/9/4/ and next to index.php and the original file, 194.jpg, there should be one additional file for each image type. Similar to this (this is for image no. 21):

    21-community-theme-default_cart_default.jpg  
    21-community-theme-default_home_default.jpg  
    21-community-theme-default_home_default_smaller.jpg  
    21-community-theme-default_home_default_smallest.jpg  
    21-community-theme-default_large_default.jpg  
    21-community-theme-default_medium_default.jpg  
    21-community-theme-default_small_default.jpg  
    21-community-theme-default_thickbox_default.jpg  
    21.jpg  
    21-Niara_cart.jpg  
    21-Niara_home.jpg  
    21-Niara_home_smaller.jpg  
    21-Niara_home_smallest.jpg  
    21-Niara_large.jpg  
    21-Niara_medium.jpg  
    21-Niara_small.jpg  
    21-Niara_thickbox.jpg  
    index.php
    

  3. 2 hours ago, datakick said:

    I've just checked @Traumflug's commit, and it did not fix the problem at hand.

    Well, it fixed the problem I found in my development installation. Looks like there's more than one problem involved here. Or I fixed something else, not the issue here.


  4. Excellent analysis! This timestamp was introduced here:

    https://github.com/PrestaShop/PrestaShop/pull/4267/commits/1e1932559758ce989a8d42d931fd650043234d21

    to fix this bug:

    http://forge.prestashop.com/browse/PSCSX-7050

    Such a timestamp doesn't make sense to me. Each image has an individual number, there should be no caching issue (unless one changes the image on the command line or by FTP).

    I've reverted this commit, which results in loading the right picture. On Bleeding Edge -> 1.1.x in ~10 minutes.


  5. On 9/16/2019 at 11:14 AM, datakick said:

    it looks to me that a large percentage of windows related issues could be fixed if the _PS_ROOT_DIR_ and _PS_CORE_DIR_ core constants were defined with the forward slash.

    This sounds like a good idea. Along with all the usages of realpath() @musicmaster mentioned. Catching issues at the root instead of dealing with them on each instance. Certainly quite some work, but not too hard to do.


  6. Alright. Now we have not one, but two solutions.

    Solution 1 is another fallback for image types:

    https://github.com/thirtybees/thirtybees/commit/ba81aad0667658f1f083f8a4a033442bbd4526d2

    This loads these images without any change to the theme or the module. Inspired by @datakick's pull request, but only for these fallbacks (I'm not sure which implications the other place might come with).

    Solution 2 is a fix in the theme, of course:

    https://github.com/thirtybees/niara/commit/df3c7f79af47ba7dde4454c990d041cc096722f8

    Yes, it's that simple. Just replace 'medium_default' with 'medium' in the theme's template. Apparently this spot got forgotten when image types got renamed.

     

    And then there are improvements like this (templates of the module and both themes):

    https://github.com/thirtybees/niara/commit/dd798ad52c103c9e97bd7cf1b0f2bc910f084b27

    which move definition of the image entirely into the template and accordingly, into the theme. Defining certain images in PHP as well as in the template doesn't make much sense. For one it's error prone, because the other definition is often out of sight. For two, only the theme template knows which image type fits the theme best.

     

    P.S.: these fixes took a while, because my first attempt was to search the best matching image type in PHP:

    https://github.com/thirtybees/blocknewproducts/commit/58c1069a7b595578f6cf3f5b792a821168e4deb3

    https://github.com/thirtybees/blocknewproducts/commit/54708c09a550233db4cc556ea93ed75138f64d47

    ... until I recognized that a module can't really know what a theme wants. Sometimes one has to implement something just to find the just written code to be suboptimal. A learning experience.

    • Like 1

  7. 4 hours ago, datakick said:

    Of course, there will be some modules that won't work with this new naming scheme. If the module developer manually concatenate strings to build image url,... well there's not much core can do to help here.

    I think the fallback can catch even these. It just needs a bit more fantasy about how module developers break code.


  8. 26 minutes ago, datakick said:

    Unfortunately that's not realistic solution. There are many modules that uses hardcoded `large_default` or `home_default` image types. These modules don't work on 1.1.0 anymore. 

    Dealing with this class of retrocompatibility would mean to try not only with appending _default, but also with removing this part of the name, right?


  9. 20 hours ago, musicmaster said:

    Try to install a fresh TB 1.10 with the Niara theme and you will see dead image links in the new products section. That module is looking for medium_default

    I see it now. The obvious quick fix is to change this name in the template. views/templates/hook/blocknewproducts.tpl, line 33. Names for image sizes were changed already.


  10. 6 hours ago, datakick said:

    I propose this pull request to strengthen backwards compatibility. @Mark, could you verify that it fixes your issue?

    Not sure which pull request you're talking about. I think the best solution is to let the module search available image types for the best match to the desired size. Another approach would be to let the module register it's own image type. This should reasonably work for all themes and arbitrary image names, then.


  11. 22 hours ago, Occam said:
    50 minutes ago, Traumflug said:
    And then there are modules entirely ignoring all this and hardcoding image names, as if there was always just one shop, just one theme and this theme always had defined image types the module wants. A static world view, just like 1990.

    For a start-up like thirty bees a statement like that sounds pretty presumptuous. Shouldn't thirty bees remain compatible for now?

    thirty bees' message isn't to stick in the 1990's. thirty bees' message is to fix bugs. That's what happened.

    Sometimes fixing one bug exposes another bug. In such cases the right thing is not to revert the bugfix, but to fix the other bug as well.


  12. 2 hours ago, zen said:

    Also there is way  too much images sizes for produits... only 3 or 4 will be enough and will help to maintain an image directory light.

    Jungle drums say, Google still ranks layouts with exactly matching image sizes higher than those using one downloaded image for all these small representations/icons.

    If you changed your theme to get away with fewer image types, don't hesitate to remove the no longer needed ones from the list. Bonus points for also removing them from the theme's config.xml (which gets read at theme installation time, only).


  13. On 9/14/2019 at 12:55 PM, musicmaster said:

    I am puzzled. Prestashop used -home until 1.4. With 1.5 it got a new theme and attached _default behind all the image formats. Are we now going back to the 1.4 naming convention?

    It simply shouldn't matter which image type names a theme uses. There's a database table storing these image types and the whole point of this table is to allow changing these names. A theme can define any image type name it feels a need for. This was "always" the case.

    Since 1.1.0, the theme name gets prefixed to this image type name. Code for this was also "always" there, it was just entirely broken.

    With this code being fixed, thirty bees allows now distinct image types in distinct themes. thirty bees features multishop, each shop can have a different theme and each theme can have its own idea of image sizes. Image type 'home' in theme 1 can have an entire different size than image type 'home' in theme 2, after all.

    And then there are modules entirely ignoring all this and hardcoding image names, as if there was always just one shop, just one theme and this theme always had defined image types the module wants. A static world view, just like 1990.

    Care was taken of these hardcoded image types as well, of course. Requests for images not existing on disk get forwarded to PageNotFoundController, which then tries to find the best match actually existing. Last resort of this controller is to send this camera image, so in the case discussed here it apparently gets activated as expected.

    Apparently there's a situation possible where the fallback mechanism doesn't find what it could find. Debugging can start in these two locations:

    https://github.com/thirtybees/thirtybees/blob/1.1.x/controllers/front/PageNotFoundController.php#L57

    https://github.com/thirtybees/thirtybees/blob/1.1.x/classes/ImageType.php#L241

×
×
  • Create New...