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

WebP issues #49

Closed
kamsar opened this issue Feb 1, 2019 · 5 comments
Closed

WebP issues #49

kamsar opened this issue Feb 1, 2019 · 5 comments
Labels

Comments

@kamsar
Copy link
Owner

kamsar commented Feb 1, 2019

While testing #43 I ran across a number of issues in the WebP implementation. @Antonytm would you mind taking a look at these?

WebP images are re-optimized on every request

Reproduction:

  • Install Dianoga from latest source, enable WebP.config
  • Upload an image that is a WebP or convertible to one to the media library
  • View the image on the frontend (ie. /-/media/foo.jpg) in a WebP-supporting browser
  • Force-refresh the image in the browser a few times
  • The Sitecore logs will show a re-optimization of the webp for each request (possibly due to the extension changes in #43 Fix issue. Save right extension of file in media cache. #45?)

Expected: the WebP should remain cached just once and be reused

WebP images converted from other formats are not actually served as WebP

  • Install Dianoga from latest source, enable WebP.config
  • Upload an image that is convertible to WebP, but is not WebP already to the media library
  • View the image on the frontend (ie. /-/media/foo.jpg) in a WebP-supporting browser
  • The Sitecore logs will show a optimization of the image into WebP, and a .webp file will be in the MediaCache. However, this is NOT the data returned to the browser, which receives a full-sized original image with non-WebP MIME types (i.e. image/jpeg)

Expected: I should get the WebP cached version if the browser sends image/webp in the Accept header, and the original version if that is not present.

It's worth noting that optimization works just fine with the cache as long as the WebP.config patch is disabled.

@kamsar kamsar added the bug label Feb 1, 2019
@Antonytm
Copy link
Contributor

Antonytm commented Feb 5, 2019

Will look on it

@GoranHalvarsson
Copy link

How is the status with this one?

@izharikov
Copy link

izharikov commented Jun 28, 2019

Faced with the same issue on Sitecore v8 (not sure, it will work on other version, including v9, but hope so).

Short Solution

Add patch config file:

<configuration xmlns:patch="http://www.sitecore.net/xmlconfig/">
    <sitecore>
        <mediaLibrary>
            <requestProtection>
                <protectedMediaQueryParameters>
                    <parameter description="Extension" name="extension" />
                </protectedMediaQueryParameters>
            </requestProtection>
        </mediaLibrary>
    </sitecore>
</configuration>

Explanation

class MediaOptions got method GetCacheKey:

public virtual string GetCacheKey()
    {
      Assert.IsTrue(MediaOptions.KnownOptions.Count == 9, "The cache key logic must be updated when known options are added or deleted.");
      string str = string.Format("as={0}&bc={1}&h={2}&iar={3}&mh={4}&mw={5}&sc={6}&thn={7}&w={8}", (object) this.AllowStretch, (object) this.BackgroundColor.ToArgb(), (object) this.Height, (object) this.IgnoreAspectRatio, (object) this.MaxHeight, (object) this.MaxWidth, (object) this.Scale.ToString((IFormatProvider) CultureInfo.InvariantCulture.NumberFormat), (object) this.Thumbnail, (object) this.Width);
      StringList stringList = new StringList();
      foreach (string key in this.customOptions.Keys)
      {
        if (!MediaOptions.KnownOptions.ContainsKey(key) && (!MediaManager.Config.RequestProtection.Enabled || MediaManager.Config.RequestProtection.QueryParametersToProtect.Contains(key) || MediaManager.Config.RequestProtection.CustomQueryParameters.Contains(key)))
          stringList.Add(key);
      }
      stringList.Sort((IComparer<string>) StringComparer.InvariantCulture);
      foreach (string index in (List<string>) stringList)
        str = str + "&" + index + "=" + this.customOptions[index];
      return str;
    }

Which is used to define cache key for compressed image. Adding new protectedMediaQueryParameters will lead to separate cache key between old-versioned and WebP images.

@squadwuschel
Copy link

how is it going forward with webp ?

markgibbons25 added a commit to markgibbons25/Dianoga that referenced this issue Feb 16, 2020
Upgrade libwebp to 1.1.0. Change to use cwebp defaults.
Add better support for webp files in media library.
@markgibbons25
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants