If an image file is not found at display time, theme_imagefield_image() generates a "File not found" comment which also contains the filename.

Due to the oddities of the SGML comment rules, this will break the page display in some browsers, should the filename contain an odd number of double-hyphens.

Comments

jweowu’s picture

StatusFileSize
new543 bytes

A patch to replace '--' with '[double-hyphen]' in any such comments.

Procedure for testing:

1. Upload an image with a single instance* of a double-hyphen in its filename into an imagefield. (* or any odd-numbered multiple).
2. Check that the page displays.
3. Delete the actual file from the Drupal filesystem.
4. Check that the page is broken in any browser which respects SGML comments (such as Firefox 3.6)
5. Apply the patch.
6. Check that the page now displays normally (aside from the missing image, of course).

jweowu’s picture

Status: Active » Needs review
quicksketch’s picture

Status: Needs review » Closed (fixed)

Seems that this is no longer necessary, because SGML comments have been deprecated and recommended to be removed from all browsers before they ever even got off the ground. Per the article referenced above: http://www.howtocreate.co.uk/SGMLComments.html

jweowu’s picture

Status: Closed (fixed) » Needs review

Sorry quicksketch, but this issue is still current and active.

You are undoubtedly aware that web developers don't have the luxury of assuming all standards/recommendations and recommendations will be implemented correctly and immediately by all browsers; and even after the new browsers implement the change, a great many people will continue to use outdated browsers for a long time to come (users in office environments frequently can't upgrade, even if they want to!)

The problem doesn't suddenly go away because someone recommends that it ought to. Developers should continue to use SGML-compliant comments as a safeguard. In time, the recommended change will provide a good measure of protection to the developers who didn't realise the problem existed in the first place, but it will be a long time before we can consider it a non-issue.

quicksketch’s picture

Okay, seems that browsers affected are:
- Opera 5-6
- Firefox ?-3.5
- Safari (none)
- IE (none)

So a good number chunk of the internet still (10-20%). As with most of the internet, I think this is a sort of a ridiculous situation. I'd prefer just to take out the comment entirely rather than accommodate for such a silly standard.

jweowu’s picture

Yeah, the reality of web development leaves a whole lot to be desired :/

How about simply including the <img> element as if the file was there, and adding a plain "ImageField file not found" comment next to it.

The worst that can possibly happen then is that the end-user sees some kind of broken image indicator in their browser (which isn't really a big deal, to my mind), and the site developer still gets all the same information they did before, because the image path is still available to them in the source.

The server gets additional requests for a file that isn't there, admittedly, which is what you'd avoided, but I wouldn't have thought this situation would crop up enough for that to be too much of an issue?

quicksketch’s picture

Yeah my preference would be to just show a broken image also. I think the entire purpose for the comment is because the file can't be read to generate the height/width properties on the image tag. There's not much purpose for an HTML comment at all if the image is missing, it's already going to generate an Apache and Drupal watchdog entry for a file not found.

quicksketch’s picture

Status: Needs review » Fixed
StatusFileSize
new727 bytes

I've committed this patch which just shows the broken image.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.