Image focal point cannot be changed on editing an image

Again, I cannot reproduce. I am able to edit the focal point as much as necessary. The symptoms you describe make it sound like the focal_point.js file is not being loaded when you try to edit which makes me think you have some other js error. Can you please confirm or deny...

Aha! .... I am seeing this issue in FF

hmmm ... I saw it one time.

Now I cannot reproduce again, in chrome, safari or FF.

Are you seeing any js errors when this happens?

I think this may be the source of the issue....

Please give it a whirl and let me know

I committed the patch in #7 and have had no problems with editing. Please reopen if this is still a problem.

Is this why I'm now running into issue like this...

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '435' for key 'PRIMARY': INSERT INTO {focal_point} (fid, focal_point) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1); Array ( [:db_insert_placeholder_0] => 435 [:db_insert_placeholder_1] => 48,46 ) in drupal_write_record() (line 7170 of /mnt/drive2/www/vhosts/himom/htdocs/includes/common.inc).

I'm no longer able to save stories that have Focal Point on them.

I fixed this PDO about 30 minutes ago ... http://drupalcode.org/project/focal_point.git/commit/95d7b29

I am still getting the duplicate entry error shown in #9, and am unable to save content with Focal Point. I've confirmed I have the latest, including the patch in #10.

Can you please post your steps to reproduce. Have you tried on a vanilla drupal install? I am unable to reproduce

I get the error anytime I try save a node with an image that has focal point.

Where I'm at right now-
I am on Drupal 7.26, and I noticed that Drupal 7.25 introduced some changes to file presave hooks. Not sure if that's related or not. But that sent me down a rabbit trail that appears to have worked.

I noticed that in focal_point_file_presave(), focal_point_save() was being called. This didn't make sense to me, so I commented it out, and things are working again. Basically, if I remove the focal_point_file_presave() function, everything works fine. Focal point is saved, and changes the crop of the images.

What is the focal_point_file_presave() function doing? Can I remove that without messing something else up?

The focal_point_file_presave function is used for media module integration. If you are only using standard Drupal fields than this function can be safely commented out. That said, Your info in #14 should be useful in debugging. I will try again soon.

jamessw, are you using a standard drupal image field or are you using a media field? Any chance you can post a feature module with your content type so I can see all your field settings?

Involves other modules but in case this is helpful -

In the media browser window, I am observing similar behavior for both Focal Point and Imagefield Focus: on initial upload, the module UI loads correctly and image focus can be set. On edit, neither JS UI (crosshairs or drag-to-create-box) appears.

For Focal point, the text coordinate box does appear as usual on edit, which suggests non-loading JS as mentioned in #1.

Both Focal Point and Imagefield Focus work correctly via the file edit pages.

The media module has this issue on WYSIWYG fields which work on file edit but not in the popup browser:
https://drupal.org/node/1508528

Just investigating whether Focal Point is the cause here (at least in all cases).

I only get this behavior with the test drive image that comes with the module! I did have some issues enabling/disabling/uninstalling module from drush because of file permissions under www-data owned sites/default/files and I just deleted the file from drupal interface then installed the module from modules page rather than drush.

No js error messages and I get the behavior on file entity edit page. Image is used in two nodes through image field with media widget.

@shadysamir can you open a different issue for the uninstall permissions issue you were seeing with details on exactly what was going on

Regarding the original issue of non-updating images, this can "appear" to occur thanks to client-side image caching. For example, if the path to your styled image after the initial edit was /sites/default/files/styles/profile_thumbnail/public/profile-pictures/your_profile_pic.jpg?itok=j5jrS_hV, the image style path (including the itok value, introduced in Drupal 7.2) will remain the same during subsequent edits and the browser may not bother downloading the image again (depending on various browser and server configurations).

I can't think of the best way to fix this (in code) that would be cross-browser-compatible or futureproof. Some people (http://drupal.stackexchange.com/questions/111404/how-do-i-add-a-cachebre...) have resorted to modifying/adding GET parameters (much like this module adds a ?/& GET parameter with the focal point on the preview page), but I don't know if this will always be supported across all browsers (apparently GET parameters are not part of any official spec for browser caching so ideally the path itself should be changed - http://stackoverflow.com/questions/118884/what-is-an-elegant-way-to-forc...). I believe having ?itok= appended to URLs has already forced Chrome browsers to never cache our styled images to begin with.

Using mod_pagespeed for auto-versioning seems to introduce as many potential problems as it solves (https://www.drupal.org/node/961510).

I'm having a similar problem. I'm running Drupal 7.35, hosted on Pantheon, and installed Focal Point 7.x-1.0-beta4. When uploading a brand new image, setting the focal point works as expected. However, any further edits to that same image's focal point produce no visible changes. I've dumped browser cache and everything, and it just doesn't change.

The only way to get it to regenerate a new cropped image is to go into Media -> Image Styles, edit the style and then click 'Update Style'. This will force ALL images attached to that style to regenerate. Then, the new cropping will appear. This is less than ideal, though.

Also, I should note that the built-in "test drive" seems to have no effect whatsoever, no matter what focal point I pick. The output image is always exactly the same.

Update: I also just installed a brand new Drupal installation on Pantheon and installed Focal Point. Using the built-in article type, it cropped fine on a newly uploaded image. Subsequent edits had no effect. Only updating the image style and regenerating did it change anything.

Update: I also just installed a brand new Drupal installation on Pantheon and installed Focal Point. Using the built-in article type, it cropped fine on a newly uploaded image. Subsequent edits had no effect. Only updating the image style and regenerating did it change anything.

Thanks for this. I assume that you didn't install any other modules (e.g. Jquery Update), right?

I wonder if there could be some server specific issue going on here. I have never seen this issue locally nor have I seen it on Acquia Cloud Hosting nor have I seen it on a cheesy shared host I have over at Dreamhost. Also (I haven't tried in a while) but I cannot reproduce this on simpleytest.me.

Anyone else who can reproduce this, were you also on Pantheon?

Yeah, I've already opened a support ticket explaining the issue to Pantheon. Specifically, I want to know if they're doing anything with caching that could be affecting it. I know they use Varnish by default, but I don't know enough about it to form an opinion.

When you install Drupal on Pantheon, you also need to install Pantheon-specific modules, which presumably just make integration into their dashboard easier. Just for giggles, I'm going to do a blank install without those modules and see what happens. I'll also do a blank Drupal install on my own Dreamhost account as a sanity check. Will report back soon.

We are seeing this on Acquia Cloud also - although at least sometimes, edits do show up for logged-in admins.

I installed a fresh Drupal 7.35 on my Dreamhost account, installed Focal Point and it worked right out of the box. Even the Test Drive worked fine.

So this is clearly something that is being affected by something Pantheon is doing. My ticket has been assigned to an engineer, so hopefully they'll have some insights.

Hi bleen18,

Could you confirm the browser you are using? As I posted previously, the clientside issue shouldn't be seen in Chrome since images with GET parameters aren't cached to begin with. A vanilla Firefox setup with caching on should be affected unless you have mod_pagespeed installed or your images paths are changing.

If using Firefox I'd be more surprised if it did work since I believe you'll see this issue with other websites as well (maybe even with mini profiles in drop down notifications - but that may be server side).

If everyone could check their image paths and itok parameters each time and state their browser we might eliminate some cases.

@cmonnow I have tried this in Firefox 36.0.1 (just tried again) with no problems.

My image location is ../default/files/styles/square/public/field/image/200927093016-14830.jpeg?itok=hL_AS71C

Not sure what you mean by itok parameters...

Thanks @bleen18.

I'm guessing then it's the way Drupal handles its $headers before sending them. Unless you've overriden them in an apache2 configuration or .htaccess files using the apache2 expires or header modules, I'm guessing it's an issue with another module we've all installed?

Without mod_header and mod_expires installed (I'll have to double check later), in my case I'm loading a profile background image on every page (via js) that gets an expiry of about 6 weeks. The problem is that validation requests are thus never sent from Firefox. Once you refresh the page with Ctrl-F5 to force validation after changing the focal point you end up getting a new file with an expiration of about 21 seconds, and every time you re-visit the page normally a validation request is sent (and you get the expected 304 Not Modified returned). Hopefully I'll investigate this soon.

(With regards to "itok parameters" I was referring to the part in bold: ../default/files/styles/square/public/field/image/200927093016-14830.jpeg?itok=hL_AS71C. I was also going to ask you to check about:config to see whether browser.cache.memory.enable is false, but now I'm guessing the cause is different.)

Hello again @bleen18!

Before I go on a wild goose chase...

In your image response headers (in the Firefox development console) are you getting Etag: and Last-modified: values being returned? And are you actually using an Apache server?

It turns out these initial values were being set by Apache (and weren't being overwritten in any way by Drupal). I also finally found out where Firefox was grabbing its seemingly random (but not) cached expiry times from (https://developer.mozilla.org/en-US/docs/Web/HTTP/Caching_FAQ). Pretty much you add 1/10th of the age of the file to the current date.

So that's why maybe if you just uploaded an image and quickly changed the focal point you could see it soon after (i.e. a tenth of the time of how long its existed) but not if you changed the focal point on an older image (e.g. from the day before). If you do a hard refresh (Ctrl-F5) after updating an image focal point it appears Drupal does set headers for the image though the first time around (Expiry, Cache-control) although I haven't traced how this happens.

Either way I suppose it would have been handy if Drupal allowed us to add timestamps to image style URIs by default so we could add expiry dates for images in our server settings as far away as possible to improve performance (in the absence of a CDN). In the meantime maybe the easiest but lower performance solution for Apache is to install mod-header and set Header append Cache-Control "must-revalidate" within an .htaccess file placed in our affected image-style's folder?

Temporary (non-ideal) solution for Apache2:

(1) If not already enabled, enable headers module in Apache2:

Type a2enmod for a list of disabled modules. If headers is present, type headers and press enter.

(2) In your cached style directory (or an upstream directory if you want to change several public directories) create an empty .htaccess file. For example, in ../sites/default/files/styles/stye-type/public/field-name/.

Enter something along the lines of this (you can make your file-type requirements stricter):

# Requires mod_headers to be enabled.
<IfModule mod_headers.c>
	# Force re-validation of image in browser 
	Header append Cache-Control "must-revalidate"
</IfModule>

@bleen18 Still waiting for more feedback from Pantheon, but in the meantime, here's a comparison of response headers from the same image on both Pantheon and Dreamhost. I don't know enough about what I'm seeing here to determine if something about Pantheon's settings might be affecting things.

http://files.atlantajones.com/focal-point-comparison.png

@cmonnow based on some of your comments above I think I need to clarify my steps:

1. Fairly vanilla drupal 7 install (local)
2. I edited an "article" with an image (note the image has been there for months in this case)
3. I moved the focal point indicator and saved the node
4. I still saw the old image on the node view
5. I did a hard refresh
6. I see the new image derivative

This is the expected behavior. You should *not* need to flush the image derivatives in order to see the new image but you may need to refresh your local cache...

/me wonders if varnish has anything to do with this?

@bleen18 It's definitely high on my list of likely culprits. Hope they get back to me soon. After having problems with Focal Point, I tried a couple alternatives, and had the same issues with them as well. Just worries me for future modules I might need.

$bleen18: Ahh, great. So we're seeing the same expected behaviour.

Some of the others (@ajBNR, and possibly @lzimmerman) definitely appear to be having an issue with reverse proxies like varnish. I don't use it but I understand you have to configure the cache expiration and purge modules to send a purge request to varnish after your entity cache is reset (and image style deleted) by this module.

Since no hook is called following image_path_flush (since file_unmanaged_delete() is used) I suppose you could call purge_urls() from the purge module manually in function _focal_point_flush?

function _focal_point_flush($fid) {
  if ($file = file_load($fid)) {
    image_path_flush($file->uri);
    if (module_exists('purge')) {
	    global $base_url;
	    $flushed_urls = array();
	    $styles = image_styles();
        foreach ($styles as $style) {
          $image_path = image_style_path($style['name'], $path);
          $absolute_path = $base_url . '/' . variable_get('file_public_path', conf_path() . '/files') . '/' . file_uri_target($image_path);
          $flushed_urls[] = $absolute_path;
  }
	purge_urls($flushed_urls);
    } 
  }
}

Note that I've never used Varnish or the purge module and I'm just basing this from a quick read of code so it might not work properly at all. I don't know what the paths should look like, whether they should exist before purging or whether image style ?itok= parameters matter at all.

For Acquia Cloud specifically you could try configuring the recommended module (https://www.drupal.org/project/acquia_purge) but again I've never used it nor Acquia for that matter.

@cmonnow I installed both the Purge module and Cache Expiration module, which Purge depends on. Implemented your code and confirmed that $flushed_urls contain the correct files. Unfortunately, calling purge_urls() seems to have no effect on the cached image. Not sure if I also need to do something on the Pantheon side, if I even would have access to do so. Thanks for that snippet, though!

FIXED! So, without jinxing myself, I think I got this licked on Pantheon.

Turns out, the Pantheon API module includes this hook:

function pantheon_api_image_path_flush($path) {
  $styles = image_styles();
  $paths = array();
  foreach($styles as $style) {
    $image_path = image_style_path($style['name'], $path);
    $paths[] = file_uri_target($image_path);
  }
  if (count($paths) > 0 && function_exists('pantheon_bulk_file_delete')) {
    pantheon_bulk_file_delete($paths);
  }
}

So I didn't realize it was running when image_path_flush() was called within _focal_point_flush(). Then after a lot more testing, I realized this hook was just referencing the image path wrong.

I changed this:

$paths[] = file_uri_target($image_path);

to this:

$paths[] = '/' . file_uri_target($image_path);

And at least as far as I can tell, this has fixed things. I've made repeated changes to the focal point and the changes always show up.

Now to just inform Pantheon of this issue that's eaten up 2 full days of my life :)

@ajBNR Well that sounds promising. Good luck!

Pantheon Pressflow 7 hacks the image_path_flush() function within core image module to call the function pantheon_api_image_path_flush().

Since no hook exists in unhacked Drupal core maybe this module should create a custom one after we call image_path_flush() for future proofing and non-Pressflow builds. If we got my code working we can use that in an optional implemented hook (or create a custom plugin for the cache expire module I suppose). Other focal point-like modules would need a similar hook.

I assume you implemented the Purge module's vcl changes for Varnish 3 in https://www.drupal.org/node/1823978#purge? I also wonder if absolute paths are necessary for the purge module?

@ajBNR ... I'm glad this fixed it for you, but I have to say that the solution you posted looks wrong. You really shouldnt need to prepend a "/" to your image paths. I'd be very curious to hear what Pantheon has to say about your findings. Perhaps they are doing something weird in pantheon_bulk_file_delete() that I'm just not aware of...

As for @cmonnow's suggestion in #36, I dont really want to put any code specific to the purge module in here. For one thing, there are several modules people might be using to deal with varnish (purge, acquia_purge, expire, varnish) and for another calling image_style_flush() really should be enough. We use varnish heavily and have never seen this problem. That tells me that if varnish is the culprit here that it is a matter of configuration...

Thoughts?

@bleen18 Yeah, unfortunately I don't have varnish installed (and don't wish to tamper with my well-oiled machine just yet) but it would be interesting to know the configurations people have used to get it to work in different scenarios (not just this module). I've seen quite a few clones of this cropping issue elsewhere (e.g. https://www.drupal.org/node/2361545 and https://www.drupal.org/node/2425149). Similar to my suggestion in #39, EPSA Crop introduced a hook recently so you can purge using whatever mechanism you like. It would be interesting to know what mechanisms can be used to purge specific image styles where image_path_flush() wasn't overwritten.

BTW, I assume you meant "image_path_flush" and not "image_style_flush" (I often confuse them myself)? The latter would be quite extreme just for editing a single image.

@bleen18 I thought it was odd, too, but changing that one thing got it all to work. Makes me wonder how it got past their QA. I've logged it as an issue on their Github repo, so hopefully I'll get some feedback. Meantime, Focal Point is working :)

PS: I ended up removing both the Cache Expiration and Purge modules. Now that the Pantheon hook is working, that did the trick.

The Pressflow 7 for DROPS code has changed (https://github.com/pantheon-systems/drops-7/commit/995f9a256abfa8cfad0d0...) how the path is created (used to have a slash that preceded the raw public:// URI) only recently. I'm guessing Pantheon QA isn't as stringent as Drupal core (which itself still has some frustrating bugs (e.g. PostgreSQL index length) unresolved from half a decade ago).

I smell varnish. Having same problem on Acquia cloud and purging varnish cache helps.

I dont know Varnish that well, but I suspect that people seeing this issue do not have Varnish configured to include query strings when handling requests for images. This would make sense as to why clearing varnish is that only thing that helps

FYI, just got this from Pantheon:

We isolated this to a regression introduced 20 days ago. Thanks a ton for your patience and diligence. A fix will be rolling out in the next few days.

@ajBNR ... if they'd be willing to share the nature of the regression, that would be awesome. Was it a varnish config? A pantheon-specific module change?

They didn't give much more detail than that, but I'll watch the Github repo for the update.

The change I linked to in comment #43 above appears a likely candidate based the changes you made to make it work.

-    if (strpos($image_path, 'public://') === 0) {
-      $paths[] = str_replace('public://', '/', $image_path);
-    }
+ $paths[] = file_uri_target($image_path);

Is there anything more to do here? I think we can safely close this issue and blame pantheon & varnish...

Finger pointing FTW

Yeah, sorry I didn't follow up. They confirmed the bug was exactly what I thought it was (from #38 above). They've since merged in the fix and sent me a free t-shirt for finding it :)

Thanks again for the great module. My client loves it.

AJ

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '435' for key 'PRIMARY': INSERT INTO {focal_point} (fid, focal_point) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1); Array ( [:db_insert_placeholder_0] => 435 [:db_insert_placeholder_1] => 48,46 ) in drupal_write_record() (line 7170 of /mnt/drive2/www/vhosts/himom/htdocs/includes/common.inc).

The above issue can be fix by upgrading focal point module to focal_point 7.x-1.0-alpha4.

Hi! First of all, thank you for this wonderful module, it provides a huge usability improvement in a very streamlined fashion.

I'm using it along with Media and even if the module works great, I'd like to let you know a common case where it fails to save the changes.

Inside the node add/edit page, the Media module displays a widget that allows you to reorder images for fields that can have multiple values, and the widget properly enables the focal point facilities. However, when trying to change the position of the focal point directly in the node form, the module fails to save the new position. You must click on the image edit button (which opens a modal window with the image-specific edit form), do the changes there, then save the image form. Even if the new position is not immediately applied to the image in the Media widget, the new position is preserved when saving the node. I've attached a screenshot to better illustrate the issue.

I think that having the possibility to change the focal point directly in the Media widget would be absolutely perfect. Right now, users have to click on the edit button of the image, wait for the modal to load, change the position, save the changes in that form, wait for the changes to be saved, then repeat with the next image. For nodes with +20 images the process is obviously counterproductive and cumbersome. I'm using:

Drupal 7.54
Media 2.0 (latest stable version)
Focal Point 7.x-1.0+5-dev (2017-02-15) with the last patch (#23) from Settings per image field