It would be nice if there were fields for definable defaults for ALT and Title text as there is in the Drupal 6 version.

Is this something that you would implement or has a conscious decision been made to exclude it? I ask as I wouldn't want to waste my time looking at writing a patch if you specifically don't want the feature.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

quicksketch’s picture

No, the D5 version doesn't include it simply because it was never implemented. The D5 version is now officially "maintenance only" so new features won't be added to it. If a patch were provided and the D6 upgrade path proven to work, I'd gladly accept a patch to provide a backport.

pribeh’s picture

bump. I'll help test.

Deciphered’s picture

Assigned: Unassigned » Deciphered

@pribeh

Good to hear.
I'll try to get a patch up and running ASAP.

Deciphered’s picture

Status: Active » Needs review
FileSize
5.37 KB

Patch good to go.

Haven't tested the upgrade path yet, but core functionality seems to duplicate Drupal 6 version.

Deciphered’s picture

Tested an upgrade from D5 to D6 and it went through without an issue.

pribeh’s picture

Can't see anything yet after having applied the patch to the 5x-2.6 version. Excited to get this going though. Where should the alt text be showing up? Is it configurable yet in the "edit field"; does it show up when adding a post within the image fieldset?

Deciphered’s picture

@pribeh

In manage field there should be ALT Settings and Title Settings Collapsible fieldsets, as there is in the D6 version.

pribeh’s picture

Wow,

Sorry Deciphered, I forgot to upgrade from D5.15 to 5.16. Upgrading fixed my issue.

pribeh’s picture

Deleting irrelevant post.

Deciphered’s picture

@pribeh

That particular support request/issue that you are having is not relevant to this particular patch, as if it's a problem now, it was a problem before.

Please make another issue for that.
I don't mean to be cold, but if this issue gets bogged up with irrelevant posts, this patch will never get reviewed/committed, and I'm relying on it for a Drupal 5 version of ImageField Tokens.

Cheers,
Deciphered.

realityloop’s picture

Status: Needs review » Reviewed & tested by the community

#4 Patch applied cleanly RTBC

quicksketch’s picture

Status: Reviewed & tested by the community » Needs review

Could anyone confirm, this looks like a problem when setting the value of the ALT field when custom alt is not enabled:

+          '#value' => isset($field['widget']['alt']) ? $field['widget']['alt'] : $file['filename'],

If custom alt is not enabled, $field['widget']['alt'] will be an empty string, so it will always set the ALT text to an empty string instead of the filename. The same problem exists for title. Neither of these things are particularly bad though, it just seems like unexpected behavior. #193887: Accessibility enhancements makes the valid point that ALT text should never be generated automatically, though I think it'd still make sense to set the title to the filename if custom titles are disabled.

Either way, the logic should be clarified. As it is now it looks like it should use the filename as the default, but if custom ALT or title is disabled, then the filename will never be shown, since $field['widget']['alt'] will be an empty string. Something like this would be more appropriate:

+          '#value' => isset($field['widget']['alt']) ? $field['widget']['alt'] : '',
Deciphered’s picture

Damn it, you're right. Rewriting patch.

Deciphered’s picture

Reworked the patch.

If custom Alt is empty it won't fill it, but if Custom Title is empty it will insert filename.

quicksketch’s picture

Status: Needs review » Closed (won't fix)

Well, unfortunately I don't think this will ever be added. I'm no longer even touching 5.x these days, and the entire D5 version will be EOL in 4 months anyway.

Deciphered’s picture

Status: Closed (won't fix) » Closed (fixed)

That's fine, I myself will probably drop support for 5.x modules, if I ever have time to work on my modules again that is.