imceimage.module produces invalid HTML (non-unique id attributes) in Lists and Front Page

I disagree. This is very important. This makes some sites unable to validate as XHTML strict.

It would be simple, I would think, to just change the "id" to "class" since there are no classes associated with the img anyway.

I noticed that the dev version (I've been using the beta) has attempted to fix the issue, so I installed it. However, it doesn't recognize my teaser images correctly anymore when I upgrade to the dev. I get errors in my logs like so:
Feb 7 16:56:46 piperhosting drupal: http://kpbj.net|1265590606|page not found|209.222.222.222|http://kpbj.net/imceimage/verifyhttp://kpbj.net/drupalfiles/kpbj_images/2010/Cover_2302_teaser.jpg|http://kpbj.net/node/15768/edit|1||imceimage/verifyhttp:/kpbj.net/drupalfiles/kpbj_images/2010/Cover_2302_teaser.jpg

Clearly it is looking for the image in an invalid URL like this:
http://kpbj.net/imceimage/verifyhttp://kpbj.net/drupalfiles/kpbj_images/2010/Cover_2302_teaser.jpg

Attempting to update the image in a node gives an immediate alert with this in it:

An HTTP error 404 occurred. 
/imceimage/verify

Attached is a patch for the beta2 version that will get us by this issue for the time being.

Oops. A revised patch is attached that adds the correct selector for the javascript so the image will show up in the node creation settings.

This fix is long overdue to be implemented, can someone apply the patch in comment #5 and include it in a new release, along with the fix in this issue?

imceimage has changed with 6.x-1.0-beta3 and with 6.x-2.x.
The new patch applies to 6.x-2.x of today - but I will commit the same thing to the -dev branch asap :)

Can you test?

This is very important. This makes some sites unable to validate as XHTML strict.

critical is wsod, or data exploding.
Let's agree on "major".
(and I set this to "needs review")

Btw, that's big diff noise because I rename every $id to $class.
I just think that makes the code more readable.

There is actually another problem that prevents this from working as well. Can you implement the fix found for this issue so I can test v2?

I can't seem to get your patch to apply... what version am I to apply it to? (I tried 6.x-2.x-dev)

If you have the version from git, you don't apply anything :)
If you use the zip (before it is being replaced), then the patch should apply, I would think.

Ok I applied the patch, but any existing CCK images on the site only show like the attached image. I think this may be an unrelated error to the ID problem, though. No PHP errors get reported...

Also, attempting to place a new CCK image gets me this error:

An HTTP error 404 occurred. 
/imceimage/verify

Baad.
To be honest, I currently don't have this set up on any test site to play with.
The one use case I had has now moved to Drupal 7.
I still want to be supportive with these issues, and maybe sooner or later I can set up a test version of the module on another D6 site, or get the older one working again.

Can you try and figure out what is going wrong?
At least the code should be nicer to read in the 2.x branch! (I hope)

I will try.

Here is where I'm at. I still haven't found the answer, but I do know that the variable $file_path in the imceimage.module file contains the following string, and I'm sure it isn't supposed to:

/usr/local/apache2/vhosts/kpbj.net/htdocs/http://kpbj.net/drupalfiles/kpbj_images/2012/Cover_2503_teaser.jpg

Also, if I disable the image validation, on line number 319 of the imceimage.module file by changing
if(($info = image_get_info($file_path)) == false ) { to
if(($info = image_get_info($file_path)) == true ) {
then I can get the teasers to show up just fine on the site, but I can't add any on the node add page.

That $file_path variable has the root path and the full url in it somehow...

Does this help you fix it at all? I'll keep digging, but I'm shooting in the dark a bit...

/**
 * Helper function to find out file location from a URL.
 */
function _imceimage_image_to_filepath($url) {
  $doc_root = $_SERVER['DOCUMENT_ROOT'];
  $file_path = $doc_root .'/'. $url;
  return $file_path;
}

This was introduced in 6.x-3.0-beta3, afaik.
I suspect that in some cases, it is called with an absolute url, in other cases it is called with a system path.
I suppose that Drupal (filefield, whatever) knows something more sophisticated for this purpose.
As a naive solution, you could try sth like:

if (preg_match('#^http(...)#', $url, $m)) {
  // absolute url, chop off the domain
  ...
}
else {
  // relative url, append to doc root
  ..
}

You could also check where the function is called, and why it gets an absolute url, instead of a relative one.
You can hardcode your site domain, for this test.

From what I can see, it is called here:

1) imceimage_verify()
That's a menu callback. The url is imploded from the path fragments - that's totally a relative path.

2) imceimage_field('validate', ..)
An item coming from the database, where $item['imceimage_path'] might hold an absolute url.

3) theme_imceimage_image($path, ..)
Called from _imceimage_image_html(), again with a $item['imceimage_path'].
(and in case of empty, we set a blank image prefixed with base path..)

(Heck, why is this a theme function anyway? not my fault, i guess)

Could you look in your database, what imceimage stores for your fields? Absolute vs relative urls

Every imce CCK teaser image in my database is the complete full absolute url.

Interesting. For me that is not. Instead, I have paths like
/sites/*/files/...
beginning with a slash.

Can you
- Try what happens if you store a new image? Will that be stored with relative, or with absolute url?
- Figure out which part of imceimage.module is responsible for writing stuff to the database? Maybe that is imce's fault?
- Rewrite the function _imceimage_image_to_filepath() to work with any kind of path, be it absolute url or relative or whatever. OR find a function that already exists in Drupal, which does the same.

I think we need to expect anything to exist in the database, but still we should agree on one way it is supposed to be stored - either relative, or absolute, with or without slash.

I'm off for today ..

Attached is a patch that brings full functionality to the 6.x-2.0-dev (2011-Dec-07) version. I had to sacrifice the initial image validation, but please take a look and see what you think. I found a fix for the 404 file not found errors, and my patch actually doesn't need to change anything in the JavaScript end. (You had a change to the .js file in your patch that wasn't really needed)

Let me know what you think. See if you can figure out why that first validation isn't working. If you think the validation isn't needed we could get rid of that I suppose, but...

  $.ajax({
    type: 'GET',
    url: Drupal.settings.imceimage.url + file.url,
    data: '',

If file.url can be a full url, then I suppose it would be better to pass it as a GET parameter, instead of appended to the path.

-------

Are you working from git?
Then we could work with feature branches, and/or I can create intermediate tags.
The -dev branch is a moving target, I don't like patching against that.

With your replacing of _imceimage_image_to_filepath(), I wonder, why not keep it in that nice function?
And, are you sure your replacement works ok if Drupal is in a subfolder? (Such that, urls are localhost/subfolder/sites/*/files/..)

Branch and tag added.
TODO is pointing the way :)

If file.url can be a full url, then I suppose it would be better to pass it as a GET parameter, instead of appended to the path.

The file.url is sent to "Drupal.settings.imceimage.url" (which is apparently imceimage/verify/) so it can get validated. I'm not sure how that part works, exactly. I wouldn't want to mess with it unless I knew I could improve it. Feel free to improve it! :-)

With your replacing of _imceimage_image_to_filepath(), I wonder, why not keep it in that nice function?

The part I replaced doesn't work in all the cases where _imceimage_image_to_filepath() is used in the module, and I didn't (don't) have time to figure out how to determine which cases have what needs... I'm not working with Git, I'm afraid. And, I have no idea if this will work when Drupal is installed in a subfolder, as I've never run Drupal that way. Can we get someone to test it?

I think, no matter what, your class changes can be implemented and released, as we know that fixes the outstanding XHTML validation problem. You could add a slash to the validation path as I did, too.
But, I have no idea why it is working unmodified for some people (apparently) at all, except that they may have relative paths in their DB, but without my additional changes, it just doesn't work with my set of modules:

Wysiwyg 6.x-2.4 with TinyMCE 3.4.9, IMCE 6.x-2.3, IMCE Wysiwyg bridge 6.x-1.1, and Imce CCK Image (modified)

Can we continue here? #820522: _imceimage_image_to_filepath() stinks. (Space in image filename causes error on save)
The invalid html has been fixed, afaik.

I'm attempting to upgrade to the recommended 2.0 version and am having a lot of trouble getting it to work. I think I finally (I hope) have it working in Firefox, but IE is still giving me the following error:

An Http error 404 occurred. /imceimage/verify

I was wondering if you were able to get this to work for you, gregarious? I already tried adding in the fix from comment #29.