This project is archived and is in readonly mode.

#2837 ✓committed
Rael Dornfest

[PATCH] [BENCHMARK] [RAILS3] image_tag shouldn't bomb on creating an alt attribute when split returns nil or ignore other parts of a filename if it has more than one period

Reported by Rael Dornfest | June 25th, 2009 @ 08:52 PM | in 2.x

Having an image filename starting with a . (odd, admittedly) causes image_tag to throw an error on attempting to auto-create an alt attribute since '.'.split('.').first is nil.

/actionpack/lib/action_view/helpers/asset_tag_helper.rb, line 493:

def image_tag(source, options = {})
  options.symbolize_keys!
 
  options[:src] = path_to_image(source)
  options[:alt] ||= File.basename(options[:src], '.*').split('.').first.to_s.capitalize

Comments and changes to this ticket

  • Yehuda Katz (wycats)

    Yehuda Katz (wycats) July 2nd, 2009 @ 07:32 PM

    • State changed from “new” to “incomplete”

    There is already a test for a case starting with period. This could only happen with image_tag("."). Is there a reason someone would want to do that? Or is there a case starting with a period that still fails?

  • Eadz

    Eadz February 24th, 2010 @ 10:41 AM

    • Assigned user set to “Yehuda Katz (wycats)”
    • Tag changed from :alt, image_tag, nil to :alt, benchmark, doc, image_tag, nil, rails3
    • Title changed from “image_tag shouldn't bomb on creating an alt attribute when split returns nil” to “[PATCH] [BENCHMARK] [RAILS3] image_tag shouldn't bomb on creating an alt attribute when split returns nil or ignore other parts of a filename if it has more than one period”

    Hi there,

    I have had a different issue with image_tag. It's bugged me for a while. I've attached a patch that fixes it. I've also included tests and benchmarks, and documentation typo fix. This makes the part of image_tag that deals with alt text twice as quick too.

    The changes are minor, and only effect filenames with more than one period.

    A filename of 'google.com.png' will now have an alt of 'Google.com' rather than 'Google'

    A filename of '..jpg' will now have an alt of '.' rather than ''

    A filename of '.pdf.png' will now have an alt of '.pdf' rather than ''

    A filename of 'slash..png' will now have an alt of 'Slash.' rather than 'slash'

    The nil issue shouldn't happen anymore as I have removed the unnecessary(?) call to split and first. File.basename(filename,'.*') does the job of removing the filename suffix and I don't know why split was there in the first place. All actionpack tests pass as of current rails edge.

    Benchmarks: ( this is a very simple benchmark. As the new version simply removes two calls, without adding any, I don't know how it could get slower anyway ;) )

    Old Version

      user     system      total        real
    

    1.510000 0.000000 1.510000 ( 1.526783) New Version

      user     system      total        real
    

    0.820000 0.000000 0.820000 ( 0.816405) Old Version [2nd test]

      user     system      total        real
    

    0.310000 0.000000 0.310000 ( 0.313089) New Version [2nd test]

      user     system      total        real
    

    0.150000 0.000000 0.150000 ( 0.153177)

  • Eadz

    Eadz February 24th, 2010 @ 10:54 AM

    • Tag changed from :alt, benchmark, doc, image_tag, nil, rails3 to :alt, benchmark, doc, image_tag, nil, rails3, tiny

    sorry, to_s is redundant too .

    Updated patch is attched.

    The method changes from

    File.basename(src, '.*').split('.').first.to_s.capitalize

    to

    File.basename(src, '.*').capitalize

  • Eadz
  • Repository

    Repository March 12th, 2010 @ 01:33 AM

    • State changed from “incomplete” to “committed”

    (from [b27376773e8f51b03bd4cb2678764cd392455870]) simplify alt tag generation for images

    [#2837 state:committed]

    Signed-off-by: Jeremy Kemper jeremy@bitsweat.net
    http://github.com/rails/rails/commit/b27376773e8f51b03bd4cb2678764c...

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>