On a clean install, with latest dev version of the module, using 9.5.

Scale effect is not working. It is incorrectly calculating the width parameter. Example:

<img src="https://abc123.widen.net/content/e7f51665-ebbd-4fdf-97fb-08550156ab66/web/IMG.jpg?w=8256&amp;itok=OlBud4mA" width="500" height="333" alt="IMG.jpg" loading="lazy">

I don't know where the value 8256 is coming from for the width. You can see in the img tag that Drupal has it set as 500. The result is that Widen does not return an image and you get the "preview not available" placeholder. I think this might be because it exceeds a size limit for the pixel dimensions, and so refuses the request.

Issue fork acquia_dam-3340734

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

davidhernandez created an issue. See original summary.

shwetaDevkate’s picture

shwetaDevkate commented

As you can see the width is given in the img tag w=8256&

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

@davidhernandez can you share the asset's width/height and aspect ratio? I don't know why it'd be blown up so large.

The related code is https://git.drupalcode.org/project/acquia_dam/-/blob/1.0.x/src/EmbedCode...

$values['w'] = self::calculateScaleDimensions($image_properties['width'], self::getDimensionValue($effect['data']['width']), $effect['data']['upscale']);

We also have safeguards to prevent values bigger than 2048, so I have no idea how 8256 is being returned: https://git.drupalcode.org/project/acquia_dam/-/blob/1.0.x/src/EmbedCode...

davidhernandez’s picture

davidhernandez
Primary language English
Location USA
commented

@shwetaDevkate The image tag itself has 500. The url is adding 8256.

@mglaman I inspected a little more. The 8256 is coming from the original image. I looked in Widen and the has its width listed as 8256. I tried embedding the same image twice. One with the scale image effect, and one using original image.

Using original image the tag adds h and w attributes with the sizes from Widen.

<img src="https://abc123.widen.net/content/ceaf09ae-c338-4556-82d2-84866e20b240/web/IMG.jpg" width="8256" height="5504" alt="IMG.jpg" loading="lazy">

The image display in this case because no width is added to the url as a parameter. When I open that url directly, Widen returns an image limited to 2048. So it appears to default to that as a max size.

When I use scale effect I get the tag reported, which puts width in the url. The width it gets from the source, so I something is causing it to use that value instead of the width from the effect (500.)

I have other people working on a different project that are also testing acquia_dam, and encountered the same problem, so I don't think it's my specific case.

If you are trying yourself, see if you can find an image in your Widen instance that is larger that 2048.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

Weird. That's why we have this:

  protected static function getDimensionValue(int $number): int {
    return $number > 2048 ? 2048 : $number;
  }

This is happening with the image_scale image effect? Which is set to scale to 500px width, correct? Are there any other image effects applied as well?

I'm seeing that image_resize and focal_point_scale_and_crop do not guard against the > 2048 condition.

EDIT

Or the logic is broken here:

  protected static function calculateScaleDimensions(int $original_dimension, int $scale_dimension, bool $upscale): int {
    if (!$upscale && $original_dimension > $scale_dimension) {
      return $scale_dimension;
    }

    if ($upscale && $scale_dimension > $original_dimension) {
      return $scale_dimension;
    }

    return $original_dimension;
  }

If upscaling turned on or off for the image effect?

davidhernandez’s picture

davidhernandez
Primary language English
Location USA
commented

@mglaman Good catch on upscaling. When I remove that it works. Putting it back it breaks again, so it is the upscaling that causes the problem.

Confirming that I do not have any other effects in use. I'm testing with one at a time.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Title: Scale effect is setting bad parameters, does not return image » Scale effect is with upscaling setting causes bad parameters, does not return image

Thanks! retitling

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Assigned: Unassigned » mglaman

Working on this now.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

Working on this.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented

I found the flaw in the test. \Drupal\Tests\acquia_dam\Unit\EmbedCodeUrlBuilderTest::testEmbedCodeUrlBuilder has hardcoded image properties of 100x100 and verifies image styles cannot scale over 2048. It also verifies images without image styles that are larger than 2048 are adjusted.

Not what happens if the source is larger than 2048 when image style effects are present.

mglaman opened merge request !7

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Assigned: mglaman » Unassigned
Status: Active » Needs review

@davidhernandez could you test out the patch from the MR?

  • mglaman authored 3ffbdacf on 1.0.x 
    Issue #3340734 by mglaman: Scale effect is with upscaling setting causes...
mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

japerry closed merge request !7