Closed (fixed)
Project:
Ubercart Option Images
Version:
6.x-1.x-dev
Component:
User interface
Priority:
Critical
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
30 May 2010 at 16:08 UTC
Updated:
7 Oct 2011 at 20:55 UTC
Jump to comment: Most recent file
Comments
Comment #1
cbassig commentedHave you tried using the majorly patched version of the module here http://drupal.org/node/550344#comment-2997386?
Comment #2
saadyousaf commentedHi
I am working on Drupal website , am using uburcart with Option Image Module for multiple switch images for each product, i have setup every thing using screen shot of Option Image Module but when i go on product view and select differenct color option then it does't switch image , only Default image i can see
can any one help me about this problem
Thanks
saad
Comment #3
acouch commentedThis should be fixed with the latest dev version. Please re-open if this is not the case.
Comment #4
imoreno commentedStill not working for me. UC 6-x.2.2 + 6.x-1.x.dev.
Modul install OK, but no image changes with the corespand attribute.
Comment #5
storytellerjeff commented+1 I'm experiencing the same problem. Attributes are working nicely, the default image appears, but will not switch to the other option images.
Comment #6
storytellerjeff commentedAnybody? This is definitely marked as CRITICAL as it's rendering Option Images as broken. I've found in the issue cues that by disabling Ajax Attribute Calculations that you can regain functionality, but there must be a way to get the two to play together nicely?
This problem combined with the inability to make the attribute as required are two big problems. Can anyone who has the skills to help take a shot at them?
Thanks!
*correction - even after uninstalling AAC, I'm still having trouble getting new option images to show up. So, yeah, Option Images is simply not working as a module. Can anyone make this really cool idea work?!
Comment #7
Toxid commentedI've narrowed it down to line 43 in the dev version, "variable_get('uc_option_image_attributes', array());" is empty. Since that is empty, none of the images get loaded. In fact, all variable_get calls that I tested in the module are empty. And then I realized that there is no variable_set calls in the module. I'm having a hard time understanding this module, but my guess is that there's something wrong with the "uc_option_image_uc_object_options_form" (line 483) function, which should save the variables.
Comment #8
Toxid commentedI've done some more research now, the above post is on the wrong track. The 'uc_option_image_attributes' variable gets populated as soon as you've set the attribute set to be enabled on the admin settings page. The problem is simply that the source path for all option images other than the first is wrong.
I've added Drupal.settings.basepath to the javascript, and the module works as expected now. I'm attaching the new js file in a zip here.
EDIT: This js file only works in Garland theme. Appearently, this bug is theme related. When I switched theme the original js file works excellent.
Comment #9
matthemattical commentedToxid's fix from #8 fixes the issue if you have the switch effect set to fade. I've attached a patch that fixes a couple of other lines that suffer from the same issue: the non-fade switch and a few lines above where the img element is created manually for cases with no original image. I'm not sure that I see how this issue is theme related, since the file paths are being pulled from the database, where they're being saved using file_directory_path(), which doesn't include the base path.
I'm still working on getting to know the Drupal code, and this module, so I'm fairly ignorant, but I'm hoping that I can help with this. With that in mind, here are a few thoughts and questions if anyone can help me...
I haven't tested the case where there is no image to begin with (lines 51 - 54 in uc_option_image.js) because I'm not sure of how to recreate this case. Any help or suggestions?
Additionally, although Toxid's suggestion and this patch seem to work fine, does it make sense to rewrite switchImageEffect to remove the old img element and clone in one of the already preloaded ones? This would bring any other properties (title, alt) with it automatically, which could be handy for future expansion. It also solves this issue, as the preloaded elements have been themed and already include the base path in the src. Not sure if that's better or just a waste of time, but just a thought that I had that I wanted to pass by anyone else interested who might know better.
I included those thoughts because they seem to me to be related. I'm still new to submitting issues, patches, and so on, and I apologize if I've done this in any way incorrectly or rudely. If I have, please let me know so I don't make a fool of myself again!
I'd be happy to work on any or all of those changes, test and submit patches. I'm hoping to use this module in a project that I'm working on and it's already so valuable to me that I'd really like to contribute. I greatly appreciate the work and time that's already gone into it. Thank you!
Comment #10
matthemattical commentedAfter further investigation, this issue only occurs when using the 'Original' setting for image size, where the path supplied to the derivative setting is relative. Imagecache presets are supplying absolute URLs because imagecache_create_url() is being used and defaults to returning URLs that way.
Not sure if this is helpful or not, but this patch prepends the base path only in the case of the original setting, set back in the .module file, thereby not breaking the other (and probably more likely) imagecache usage.
Comment #11
acouch commentedI committed the patches from smatthew. If #8 is converted to a patch I can add it as well.
Comment #12
matthemattical commentedI made a mistake, and the patch from #9 (and the fix in #8) fix the issue for the Original image setting, but unfortunately break using imagecache presets. The patch in #10 adds the basepath to the src back when it's populated into the Drupal.settings.UCOI.derivative setting, and only if the Original setting is being used, so that both imagecache presets and the Original setting work.
Sorry for the patch in #9, which is faulty; the patch in #10 seems to fix the issue without breaking anything else.
Comment #13
cybis commentedThe last commited patch is causing the following error on my install (note the doubled base URL):
Comment #14
agileware commentedHere is a patch to fix the current issue (extra base path being added).
Comment #15
cybis commentedPatch #14 works for me.
Comment #16
acouch commentedI committed the patch from #14
Comment #17
cybis commented