Rails Ruby on Rails → 2.3.9

Comments and changes to this ticket

• 

  Rohit Arondekar June 14th, 2010 @ 08:00 AM

  • Assigned user set to “José Valim”

  Interesting.

  I found the reason why html_safe! doesn't actually do anything besides raising an exception:

  For performance reasons safe strings are implemented in a way that cannot offer an in-place html_safe! variant.

  source: http://edgeguides.rubyonrails.org/active_support_core_extensions.ht...

  i.m.h.o I think the method should be removed or at least the exception raised should explain why you cannot call html_safe! on a string.

  On that note, +1 to the patch, applies cleanly and tests pass on 1.9.2-head and 1.8.7-p174

• 

  Neeraj Singh June 14th, 2010 @ 02:33 PM

  I would say that this code is self documenting and since html_safe is something a lot of people would encounter in rail3, this method should stay.

• You flagged this item as spam.
  

  Cody Fauser June 14th, 2010 @ 02:47 PM

  Neeraj,

  I disagree. html_safe! is not a usable method on any object. There is no better way to indicate that you can't call a method than to not implement it and the method is clearly deprecated in Rails 2.3.6.

• 

  Rohit Arondekar June 14th, 2010 @ 02:55 PM

  If the method is to be kept I would prefer it if the message is more self explanatory. This will avoid confusion and people filing bug reports.

  If anybody agrees with me, I've attached a patch that should explain why the method can't be called. Tested it on 1.8.7-p174 and 1.9.2-head.

  • improve_exception_message.diff 913 Bytes
• 

  Rohit Arondekar June 14th, 2010 @ 03:09 PM

  For the record I still agree with Cody though, if the method can't be called it shouldn't exist.

• 

  José Valim June 15th, 2010 @ 12:07 PM

  In Rails 2.3.5, the official API was html_safe! However, we found out that it's actually slow and provided another implementation, so we needed to deprecate the old html_safe! method and stop modifying strings in place.

  We preferred to use RuntimeError because we wanted to be clear that the html_safe! existed but we changed its behavior. If we raised NoMethodError, developers would be wondering what happened and if they were actually doing something wrong.

  But since it was already deprecated on 2.3.6 and it shows a message on 2.3.8, it can be removed for 2.3.9.

  I hope this clarify a bit. I will apply this patch later.

• 

  José Valim June 20th, 2010 @ 11:14 AM

  • Milestone set to “2.3.9”
  • Assigned user changed from “José Valim” to “Michael Koziarski”

  Assigning to Koz to give the final word here since html_safe is his baby.

• 

  Santiago Pastorino June 20th, 2010 @ 03:58 PM

  • State changed from “new” to “resolved”
  • Assigned user changed from “Michael Koziarski” to “Santiago Pastorino”

  This is already out on 2-3-stable since this commit

  http://github.com/rails/rails/commit/60e82a3ca2fe6058292db659c34117...

• You flagged this item as spam.
  

  Andrea Campi October 11th, 2010 @ 07:20 AM

  • Tag changed from “patch activesupport xss” to “activesupport, patch, xss”
  • Importance changed from “” to “Low”

  bulk tags cleanup