Posted by sampeckham on August 11, 2008 at 4:47pm
| Project: | Image Assist |
| Version: | 6.x-3.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | TwoD |
| Status: | closed (fixed) |
Issue Summary
I've been using this dev version and the beta version and in both cases it keeps reverting to the thumbnail version of the image, no matter what size I select in the drop down list. I had the beta version working once, then it stopped for some reason. I've tried with and without TinyMCE and have been mostly using the 'HTML code' insert option.
The only way i can show any other version is by manually editing the HTML img tag produced.
Anyone else have this problem, or know of a possible cause (other than being in the dev stage!)?
Comments
#1
As for the HTML tag feature: #284447: Remove HTML output feature - it's about to be dropped, because it causes plenty of bugs.
Could you please verify whether this bug happens if you're using inline tags?
#2
OK, I've played around a bit more and this still happens when I use the Image Filter tags. I'm going back to the beta version (which seemed a bit more stable, but has the same problem) and will report any further details about this. I'm using D6.3 btw.
Also, an aside the dev version doesn't seem to work with tinyMCE, I've set it up just as the beta version and I see the camera icon, but clicking it does nothing - maybe this is expected in dev?
#3
I wondering if this is related to the Image module and dimensions. Although all size images are generated correctly, when I use just one dimension value for scaling, Image Assist reverts to the thumbnail version. If I select an image size with both values it embeds the correct size, but adds the wrong dimension values to the code.
EG.
I have Thumbnail 100x100 and Preview 300x300, and also 'full' 530x*** and 'half' 260x***
If I select 'full' or 'half' the thumbnail is used.
If I change 'full' values in Image module to 530x250 (scale and crop and regenerate image derivatives), Image Assist embeds the 'full' version (with correct file name) BUT creates dimensions of ***x250, resizing the image proportional to it's height. Not what you want to happen, as Image module has already scaled and cropped.
What I want is to upload and define two widths for images and use either full or half width image in the node.
So Image module is working and doing exactly what t should, but Image_assist is not quite reading the file dimensions somehow. Is it designed to "fall back" on the thumbnail view if it can't work out dimensions? Maybe that's what's happening.
Hope this helps understand the problem, been testing it all morning, including uninstalling, deleting all image nodes and files and downloading a fresh copy of the Beta version.
#4
We had a similar issue in the past, but that was related to Image 5.x-1.x. It is possible that something changed since 6.x-1.x (which seems functionally to be the same as 5.x-2.x).
Needs further analysis.
#5
I think the issue is still there: you have to specify both width and height in Image's settings för IA to work correctly. A workaround is possibly to use a size like 100x999 px.
#6
Bumping to critical. This issue needs to be fixed for 2.0 to be released.
#7
Here is a patch, my first img_assist patch. Watch out! :)
#8
From a first, quick peek, this looks pretty good and reasonable. 4 confirmations from other users are required though:
2x This patch does not break images on existing sites using 6.x-1.x or 6.x-2.x (until now, nothing changed in this logic since 1.x).
2x This patch works, i.e. allows to leave out either width OR height in Image's settings for a preset and the resulting images have the correct/expected size.
#9
I don't think this code handles all possible cases for whether $width and $height are set. We need to end up with both $width and $height set, in order for $diag_size_new to get a proper value on the next couple of lines.
There are four possible cases:
both $height and $width set:
1) $height larger than $width
2) $width larger than $height
only one is set:
3) only $height set
4) only $width set
I think only 3) and 4) is being properly checked. 1) is covered, but there is no check that both values are set before comparing them.
#10
Yay quick reviews. I see your zoo33 point regarding 2). Here is a new patch.
But for 1), it doesn't create a problem anyway, because if $width is 0 then it would be less than $height anyway.
Cheers,
#11
My concern is this:
$diag_size_new = sqrt(pow($width, 2) + pow($height, 2));We need both $width and $height to be correct in order to determine the diagonal size.
#12
Both $width and $height are fine. Because the whole block of code will only execute if either of the dimensions is defined (
if ($width || $height)). Then another condition (if (!$height || ...)) checks and sets the remaining dimension.I left the code running on my personal blog and so far it's working. I would happily work on a better patch if I'm able to reproduce.
Cheers! :)
#13
Hm, guess you're right.
#14
We need someone to test and confirm this.
#15
#163043: Unspecified image preset width/height: Wrong image size calculated, missing images, etc. has been marked as duplicate of this issue due to some further debugging by TwoD.
Additionally: In img_assist_properties_form() we are populating the select field for the property window, but if an image preset size does not specify a width or height, the corresponding values are empty, which completely breaks the aspect ratio calculations performed elsewhere.
With "elsewhere" I mean the calculations that happen mainly in JS. I would like to remove that JS calculation code altogether in favor of properly prepared values on the server-side.
#16
This should be all that's required for the properties form. May or may not need to be merged with alienbrain's patch.
#17
Patches in #10 and #16 confirmed. Both individually and together.
I think the JavaScript (img_assist_popup.js, onChangeSizeLabel()) should also be altered too at the same time. It should no longer [re-]caluclate the width/height values and instead just pass them on since they are now ensured to be correct from the server. But I don't see this as very important since the values should come out correct in the end anyway.
#18
Committed both patches to ALL branches - literally.
This was the first commit since months to 1.x... ;)
#19
Automatically closed -- issue fixed for 2 weeks with no activity.
#20
I've run into the same problem described by the original poster (and better described by poster #5 here: #163043: Unspecified image preset width/height: Wrong image size calculated, missing images, etc.):
Inserting image using IA, and ending up with just thumbnail in resulting page, even if I enter custom width and heigth. Going back into editing mode, I just see a small red dot instead of image. Disabling rich text and manually entering w x h works.
I have all the latest dev snapshots installed:
Image 6.x-1.x-dev (2009-apr-18)
Image Assist 6.x-3.x-dev (2009-mar-29)
Inline 6.x-2.x-dev (2009-feb-13)
Wysiwyg API 6.x-2.x-dev (2009-apr-21)
Note: For me this happens only in IE (version 6, 7 or 8) - when using Firefox everything works without any problem! I've not seen any mentioning of this beeing a browser dependant error, neither in this thread nor in any of the different duplicates.
#21
this is still an issue using 2.0 alpha 3.
#22
I've also seen this problem appear on my sites again. Same symptoms as in #20 but in Firefox too.
I think I could avoid it by selecting that small red dot (which is just the image resized btw), opening the Image Assist window, selecting image size and hitting update again.
I'll take a look at it tomorrow again, too late here now, assigning this to me in the meanwhile.
Edit: I've found the problem. So far, this seems to be specific to IE and possibly combined with TinyMCE. Will try with FCKeditor and others later.
When Wysiwyg API initiates TinyMCE, it sets up a listener on the
onGetContentevent to know when to detach plugins. In Image Assist's case this means converting the placeholder img tag back into a filtertag so the outside world will not see the placeholder. That's usually exactly what we want, but TinyMCE itself makes a call togetContentto store the initial contents of the editor, which of course triggers that event.Due to a timing issue, IE has not yet completely parsed the placeholder img tag into the DOM (or that's what I'm guessing so far). So the contents returned to TinyMCE will get a filtertag with width and height set to 0, because that's what the Image Assist plugin got back when looking at
this.widthandthis.height.thisbeing the img tag which was just recently created.I've tried various ways to access the width/height but no luck so far, except with using
/width=(\d+)/i.test(this.outerHTML);alert(RegExp.$1)(wherethisis the img tag) which does indeed work but feels a bit awkward. Sadly, I see no easy way around implementing IE-specific behaviour into the Image Assist plugin by relying on the .outerHTML property and manual parsing like that...#23
I createded a workaround patch for this. It's not as pretty as the original solution but works.
If this.outerHTML exists, use a simple regular expression to parse out width and then height and build the second part of the filtertag using the matched values.
I tried setting this.width and this.height but it wouldn't accept it. They're probably read-only at that time...
#24
Sorry, but have problem downloading patch - clicking on attachment link above only redirects me to Drupal home page.
#25
I'll try uploading it again. I got a notice before saying that my file was renamed due to security reasons. Maybe there's a bug which makes it forget to change the url as well?
I got it this time as well, had removed the # so I guess that wasn't it. Removed .HEAD. as well (noticed it added an extra _ after it) and now it seems to work.
#26
Good news is that after patching, IE8 does not mess up the image size when opening a page for editing, and I'm even able to insert a new image using IA, ending up with correct size.
Bad news is that if I try to edit the image size of an existing image using the IA pop-up, when hitting "Update" the image disappears! Well, in fact it doesn't disappear completely - it jumps to the top left corner of the page - but if I try to save the page, the image is totally gone. Going back into editing mode, the image is gone, both in wysiwyg view and in rich text disabled.
As usual there is no problems using Firefox.
#27
The bug jofje mentions in #26 has been driving me mad for a couple of hours now...
I've at least been able to determine that it also happens without my patch from #25, and I've found a way to almost always reproduce it in IE (6).
<body>tag of the main page instead of where it's supposed to. (Which is also why it disappears if you save the node, it's no longer in the wysiwyg area.)This makes me think it might be a TinyMCE problem. Maybe the caret position is completely off in IE if the last thing clicked before executing the mceInsertContent command (which is what Image Assist does) was not actually the image we intend to replace with an updated version.
A comment in the TinyMCE setContent method (called by the content insertion command) states:
I tried to verify this by getting what was selected from TinyMCE but that looked ok (the img tag was returned), so I guess selection and caret position are out of sync.
In any case, I believe this is a separate issue and not related to the patch I posted. The patched detach() method also does not run when updating the image, at which point the above problem occurs.
Btw, I also noticed that IE never seems to be able to get the width and height from the placeholder img tag, regardless of when the detachment happens.
#28
subscribe
#29
I almost forgot about this issue... Could somone confirm that the issue in #26/#27 is unrelated? I don't really like this hack (parsing out the value from .outerHTML) but it's the only thing I found which actually worked. I can't find much to add to it exceept for maybe changing the comment so it reads "#293909: IE has not specified width and height properties.".
#30
I also had the same problem, I was using Filter option and width and height weren't inserted correctly. I upgraded to the dev version 6.x-3.x-dev and the problem is no longer there :)
#31
The properties form probably still needs to be fixed.
However, we missed to update the rest of the img_assist_render_image() function in those previous patches. The diagonal image size dimension calculation fails when a default image size has no width or height or misses both (like for IMAGE_ORIGINAL). Also, the previous patches did not account the possibility that the width may be not be specified in the inline macro (we only fixed height). So, when for any reason, width or height or both are 0, '', or undefined in the inline macro, then also the diagonal image size calculation failed, resulting in IMAGE_ORIGINAL being used by default.
Attached patch should fix this. Took me some hours to wrap my head around that code.
#32
Oopsie, with debug code commented out... (I want to leave this in there to quickly be able to debug)
#33
Fixed a comment.
I've deployed this to a large-scale production site and it seems to work flawlessly. Any objections?
#34
The last patch looks good to me, and your explanation sounds reasonable. It sounds like you have tested it well enough, so RTBC i guess?
#35
Thanks for reviewing! Committed to 2.x and 3.x branches.
Now back to #26/27... fixing property window data processing in IE. (Yuck.)
#36
subscribing...
#37
subscribin'
#38
Sorry, going back to #10:
I think this only works for images in portrait format (where height is larger than width). This is because, to get to know which side is larger, you just need to compare "$width <= $height", not "round($width / $aspect_ratio) <= $height". The latter leads to "true" in many cases even if width is larger than height.
Following changes work for me (sorry, don't know how to create a patch and don't got time for it, just want to let you know):
1134 // Get new width and height for this inline image.
1135 // If height is either left out or larger than width then
1136 // Width is controlling factor.
1137 if (!$height || ($width <= $height)) {
1138 $height = round($width / $aspect_ratio);
1139 }
1140 // Else, width is either left out or larger than height so
1141 // Height is controlling factor.
1142 else {
1143 $width = round($height * $aspect_ratio);
1144 }
1171 // Calculate default width and height based on aspect ratio.
1172 // Width is controlling factor.
1173 if (!$height_std || ($width_std <= $height_std)) {
1174 $height_std = round($width_std / $aspect_ratio);
1175 }
1176 // Height is controlling factor.
1177 else {
1178 $width_std = round($height_std * $aspect_ratio);
1179 }
#39
subscribing
#40
Subscribing - #27 is frustrating me like no other....
#41
Thank you for the quick fix hoyer, it worked for me too.
EDIT: Perhaps not, with just this quick fix applied Portrait images default to thumbnail size rather than the specified one.
#42
I applied the patch from 33, and it has not fixed my problem. Probably because it wasn't intended do, but alas.
The explanation in #27is exactly what's happening to me. If I edit an inline image with img_assist from my tinymce wysiwyg, and push update, the image will appear on my page, but WAY outside of my textarea. It will appear in the body of my actual template, way out on the sides.
Has anyone found a solution to this? It's very troubling. thanks in advance.
As always, FF is fine...
#43
Subscribe.
#44
#38 solved my problem
#45
subscribe
#46
Subscribing, I think I'm seeing the same behaviour.
Sometimes when I upload an image, it doesn't respect the size chosen and inserts a link to the original. The behaviour isn't consistent, and two images from the same source can produce entirely different behaviour, with one resizing correctly and the other not. The only solution I have atm is to resize the images before uploading.
#47
Subscribing.
Same behaviour here (6.3.xdev). I created 2 custom image sizes/presets within the Image-configuration: 270x270 and 540x540, resize (no crop). Now, when I select 270x270 as desired preset in the img_assist views-interface, the preview in the wysiwyg window looks fine. As soon as I save the node and reload the page, there's not the 270x270 image showing but the default "Thumbnail" preset which is being provided by the Image module.
Generally, everything works fine until the point at which I save the node: After the node/filter-tag is being processed, the "Thumbnail" size is showing up. This makes me belief that this is actually a problem of the filters behaviour, not of the actual image editing/inserting process via JS.
What is pretty weird is, that it works for the 540x540 preset. Some array sorting something wrong at filter-time?
#48
subscribing
#49
I can confirm genox's comment for 6.x-3.x-dev (downloaded 26th Jan 2010). I've done a bit more digging:
For these image module settings:
Original: 800x800Thumbnail: 100x133
Preview: 250x333
Large: 350x467
Lanscape images respect the preset on the edit window, but default to thumbnail on the node view. Portrait images mostly respect image preset on the edit window, and always scale corectly on the node view.
For these image settings:
Original: 800x800Thumbnail: 100x<blank>
Preview: 250x<blank>
Large: 350x<blank>
Lanscape images respect the preset on the edit window, but default to thumbnail on the node view. Portrait images respect image presets on the edit page. Image assist only displays Thumbnail and Preview presets, not the Large preset. On the view page, portrait images default to the Original image size (as scaled by the Image module) and ignore the preset.
For these image settings (in response to a suggestion above):
Original: 800x800Thumbnail: 100x999
Preview: 250x999
Large: 350x999
Landscape & portrait images respect the preset on the edit page, but image assist makes only the Thumbnail preset available - none of the others are available on the image assist window. On the view page, the preset is ignored and the view defaults to the scaled original in each case.
In each case above, when you edit the page once more, after viewing it, the presets have been forgotten when you go back to the image assist window and you need to reset.
The image assist max image size is at 640x640, but none of my images would exceed these dimensions once scaled, apart from the sclaed original which gets displayed from time to time anyway.
#50
subscribing
#51
Subscribe, same as #46, Image Assist seems to ignore whatever I choose and default to a random other size. Currently it is selecting original.
#52
When inserting images inline (6.x-3.x-dev) many uploaded images would only display inline as thumbnails. I was able to prevent this by adding body text to the image node when uploading. I ended up just hard coding
theme_img_assist_inline{
$label = $size['key'];
to
$label = 'my_image_preset'
#53
I think I have the insertion size problem solved.
What I have done is taking the original image_get_sizes function from the image module and rewrite it as img_assist_get_sizes. The new function returns the actual sizes used, of course you must pass the aspect ratio in order to achieve that.
I've also modified the form to get the size and preset name in the select values so now when you insert an image you got the preset name and it's original size as preset, presetw, preseth. So when the image is shown the presetw and preseth are compared to the actual width and height in order to know if a new preset or custom size must be used.
I have tested this and is working fine but I guess there are bugs still there, I'm using only presets in the testing site, so it is not possible to create custom sizes, I mean custom sizes should be working fine but I have not tested them since my testing site doesn't allows them. I also think the IE problems are still there. I've only tested with Chromium and Firefox (Linux desktop here).
However I must test and fix IE problems since my client demands it working on IE (we developers have to carry on with Microsoft faults). This brings me to a question: Why is there javascript code that uses getElementBy.... stuff instead of jquery? I think that is what is causing the IE problems.
Anyway, here is the patch for you to please test it, it was made against the current development version on the project page: img_assist-6.x-3.x-dev (2010-jul-11).
Please let me know if something needs to be fixed.
#54
Please do not attach compressed archives to issues on d.o. Can you just attach the plain .patch file?
#55
Sorry, I didn't knew about it. I was trying to save some server space.
I have fixed the width=0, height=0 problem on IE. I had to use regexps to get the values from the code since every javascript method (jQuery's attr, getAttribute, etc.) fails on IE. Now if the attr method returns something that evaluates as false (0, undefined, ...) then the value is catch with a regexp.
However there is still a bug on IE, when you update an image a new image is added instead of replacing the old one. Even worst it is put on a different place.
I will keep working some other day on getting it fine on IE (I should start charging +50% for IE support).
#56
Hmm... >90% of this patch are changes unrelated to this issue (and also not adhering to Drupal's http://drupal.org/coding-standards). We need a patch containing just the necessary fixes in here.
#57
Well, I have edited the sources to comply with the coding standards, changed tabs to spaces and removed some commented code.
As for the changes I think they are related since what it does is to get the images shown in the right dimensions. With this patch if you choose a preset size for the image then it will be shown with that size in the WYSIWYG editor and in the final document without falling to thumbnail size. It does not fail if the image is in portrait or landscape mode or if the preset has only one dimension defined.
I think that is the main issue here.
It is consistent with the original intended behavior in that if after insertion the user changes the size of the picture (by dragging a corner or modifying the value in the code) then, when the image is rendered, the nearest preset that respects the aspect-ratio is used or a new size is created if the user who edited the node has the rights to do it.
It also fixes the problem exposed in #20 and other latter posts by which IE always gets width=0 and height=0 when using WYSIWYG. What I'm doing is that if by using the regular javascript function (jQuery's attr in this case) the dimensions gets a value that evaluates to false (0, undefined, etc.) then the value is catch from the img's html code (by using jQuery's html() method to get the code).
So, please, give it a try and see how it works.
It is working fine here for me, I am using the wyiswyg module with the tinymce editor and the img_assist extension for wysiwyg.
#58
I don't know all of the etiquette here (one of these days I'll learn), but when I edit this line in img_assist_render_image():
if (round($width_std / $aspect_ratio) <= $height_std)
to this:
if (empty($height_std) || round($width_std / $aspect_ratio) <= $height_std)
I can leave the height blank and get image assist to display the size that I selected and not the original size.
#59
Hi,
there is another thread here that might be discussing something similar: http://drupal.org/node/716872 As per my note in there, I did try the above #57 patch, but when I went to edit an inline/img_assist image in IE 8, it deleted the image once I saved the TinyMCE popup. I quickly uninstalled the patch.
Happy to test further if someone want's to look into this further.
#60
Hi parkview,
As exposed in #42 and #27 the inline image update or edition is buggy on IE. The patch on #57 deals with the width and height problems that make the image be shown always in thumbnail size (on every browser) and that make the width and height equal to 0 only by editing a node on IE.
As I told in #55 the update problem on IE remains. I'll have to work on it in the near future, because my customers demand it working fine on IE. But I've been busy in other issues.
However, did you try inserting new images and seen if they're shown with the right size?
#61
Where exactly does this patch go? I am having an issue with images popping up in IE7 and 8. They popup fine in Firefox. I am using tinymce as my editor. Please tell me the file directory path I should make this changes. Thanks
#62
subscribing
#63
Subscribing.
I'm having major problems with custom sizes. All good when I edit, but when I save, some other size than what was chosen is inserted...
Included a couple images, where you see the filter tag and the resulting image (which is half the size).
When I view the image, I also see that it has inserted another version ".thumbnail.jpg" than what I inserted (and what the dimensions in the filter tag says).
Edit:
This is the actual code it generates in the finished page ("halvside" is the default image assist-format I have chosen, not the one I chose for this picture):
#64
I'm having similar problems. Also very strange.
My install seemed to be working fine - everything as expected but then at some-point a week or so later it just stopped. All new images I placed in posts were being shown as thumbnails. I hadn't touched any image-related modules. I had been messing with some modules.
I then uninstalled image-assist and reinstalled it and it began working again. But now a few days later it has stopped again.
The only thing I can think is whether it's a memory issue. Does a shortage of memory mean the size conversion doesn't happen and it reverts to the thumbnail size?
Any other suggestions?
B
#65
One thing to add - in the text-area box in the form - when I've added the photo it shows perfectly in the form. It's only when I save or preview the content that it reverts to thumbnail.
B
#66
Me again. Sorry to post three times in a row but it has been nearly two months.
I am still completely stumped by this. Has anyone had any luck? Has anyone managed to successfully apply the patch in #57? I have been trying all day and can't get it to work.
Is no-one else getting this issue anymore? This is a popular module but for me it just plain doesn't work.
Thanks
B
#67
The patch failed to apply for me as well. I backed up to version 6.x 2.x and now it's working as expected for me.
#68
Looks like the remaining bug has been identified (and fixed) in #669092: display original size instead of preview
#69
Automatically closed -- issue fixed for 2 weeks with no activity.