processwire / processwire-issues

ProcessWire issue reports.
44 stars 2 forks source link

Pageimage::focus doesn't correctly set when using array as first argument and there is no second argument #1919

Open poljpocket opened 2 months ago

poljpocket commented 2 months ago

Short description of the issue

The focus method of Pageimage does not set correctly when there is no second argument because of a seemingly wrong isset check in the code. I stumbled upon this bug because I was trying to set the focus data of one image to all the others in the Pageimages field. By the docs, and also by the code, it should be possible to get the focus data of one Pageimage and then use the return unaltered to set the same focus data for another Pageimage.

Expected behavior

When using an array as the first argument for Pageimage::focus method, the focus data should be set.

Actual behavior

When using an array as the first argument for Pageimage::focus method, the old data (or default) remains.

Suggestion for a possible fix

The seemingly problematic line in question is this one. Also, this change here from 2 years ago might relate to this issue?

Shouldn't this check be looking at $top being an array instead of checking for $left being set? It probably should be someting like

if (is_array($top) || $left !== null) {
 // SET
 // [...]

I will not add a PR because combined with the issue above, I am not sure about the side effects.

Steps to reproduce the issue

  1. Get a Pageimage, e.g. $pimg = $page->images->first();
  2. Use something like $pimg->focus([80, 80]); which should set the focus point to 80% top, 80% left
  3. Observe the focus is still set as default or whatever has been set before

When you use the second argument, e.g. $pimg->focus([80, 80], 80); everything works. Also, the second entry in the array can be anything at this point, it just gets ignored.

Setup/Environment