This project is archived and is in readonly mode.

#2864 ✓resolved
Emilio Tagua

[PATCH] fix rendering of boolean attributes in tag helper methods

Reported by Emilio Tagua | July 2nd, 2009 @ 09:17 PM | in 3.0.2

After adding video_tag in ticket #2843, we definitely should have the audio tag.

The patch:
- Adds the audio_tag and audio_path, with test and docs. - Fixes video_path typo and error in docs. - Changes audio and video attributes values so they are either true or false and not attribute's name value. This is specified in [1] and [2].

[1] [2]

Comments and changes to this ticket

  • Emilio Tagua

    Emilio Tagua July 2nd, 2009 @ 10:04 PM

    • Assigned user set to “Yehuda Katz (wycats)”
  • Yehuda Katz (wycats)

    Yehuda Katz (wycats) July 3rd, 2009 @ 05:46 AM

    • State changed from “new” to “incomplete”

    git claims this patch is corrupted. Can you try again?

  • Emilio Tagua
  • Emilio Tagua

    Emilio Tagua July 3rd, 2009 @ 01:30 PM

    The new patch should be working :)

  • Repository

    Repository July 7th, 2009 @ 11:57 PM

    • State changed from “incomplete” to “resolved”

    (from [1e2d7229602f467cfdc0ef606b5ef8a5566a1501]) Adds a audio_tag helper for the HTML5 audio tag. Fixed video_path docs. HTML attributes values should be true or false not attribute's name. [#2864 state:resolved]

    Signed-off-by: Yehuda Katz wycats@yehuda-katzs-macbookpro41.local

  • tiegz

    tiegz July 16th, 2009 @ 07:53 PM

    Hey Emilio,

    I saw your references to W3Schools to the boolean attributes, but I think they might be out-of-date on the HTML5 specs. I left some further comments on github and wanted to get some feedback from you guys? Of course it might raise the question of Rails following the HTML5 spec totally (or just leaving things as they work in all browsers):

  • Marc Love

    Marc Love July 26th, 2009 @ 09:28 PM

    +1 For tiegz comment

    This brings the produced HTML out of compliance with the HTML5 draft specs. All prior HTML specs have treated boolean attributes in the same manner, so there's no reason to believe they'd suddenly change direction with "true"/"false" values.

    What's the preferred method for fixing something like this Yehuda? Doing a revert on the commit and creating a new patch that does not include the incorrect changes? or a new patch that will correct the errors created by this patch?

  • Pratik

    Pratik July 26th, 2009 @ 10:01 PM

    • State changed from “resolved” to “new”
  • Pratik

    Pratik July 26th, 2009 @ 10:02 PM

    Marc : Better to just create a new patch fixing the issue.

  • Lukas Westermann (lwe)

    Lukas Westermann (lwe) July 26th, 2009 @ 10:20 PM

    Left a comment on github:

    How about changing ActionView::Helpers::TagHelper#tag_options to handle boolean arguments differently? So that tag_options(:disabled => true) would render to disabled="disabled" and tag_options(:disabled => false) to "", just as in the HTML docs. However, I'm not sure what the implications would be for existing applications, that depend on the fact that currently tag_options(:foo => true) returns foo="true".

  • Lukas Westermann (lwe)

    Lukas Westermann (lwe) July 27th, 2009 @ 12:02 AM

    If anyone is interested, I've written a patch which addresses the issues described above and gets rid of the TagHelper#BOOLEAN_ATTRIBUTES array as well. Though, if there are any projects out there which rely on the fact that tag_options(:foo => false) returned " foo=\"false\"", they will break... On the other hand the method proposed by the patch is (in my humble opinion) much more logical and can be used to do some stuff like: audio_tag 'some.mp3', :autoplay => @user_preferences.autoplay?, which will either omit or define the attribute "autoplay", based on a boolean value.

    PS: added some more tests as well, to check for the escape option and adopted some more tests (forms etc.) to use boolean values.
    PPS: As described above, I've no idea if this patch breaks anything else, the tests run fine though :)

    Btw - if somebody used to do text_field_tag 'foo', :disabled => 'yes' it will return invalid HTML though... honestly, I think it's ok like that, boolean attributes should be used with boolean values. FYI - in fact some tag helpers depend on a boolean value being passed (i.e. select_tag for :multiple), otherwise they might not function correctly anyway.

  • Yehuda Katz (wycats)

    Yehuda Katz (wycats) July 27th, 2009 @ 01:41 AM

    The HTML5 spec specifically disallows true/false and allows the lowercased attribute value or empty string only. I'd say write a patch against current master, rather than reverting

  • Marc Love

    Marc Love July 29th, 2009 @ 11:17 PM

    Lukas, I don't understand what the need is for eliminating the #BOOLEAN_ATTRIBUTES array. Your patch introduces new, unexpected functionality for HTML boolean attributes and I don't see where you've delineated the need to do so.

    I'm going to add a patch here in a second, against the master as suggested by Yehuda, that will simply return the expected functionality of boolean attributes to what they were before this patch was applied. If you really think its important to change it to the way it is in your patch, I'd recommend creating a new ticket explaining the need, attach your patch and get community feedback/review to see if such a change is deemed necessary.

  • Marc Love

    Marc Love July 30th, 2009 @ 12:08 PM

    • Title changed from “[PATCH] add audio_tag to ActionView” to “[PATCH] fix rendering of boolean attributes in tag helper methods”

    Attached is the new patch as requested, Yehuda, which fixes boolean attributes to follow the XHTML/HTML5 standards.

  • Repository

    Repository July 30th, 2009 @ 05:48 PM

    • State changed from “new” to “resolved”

    (from [d860c8917023e62d24c239bbc9aa6c629704f416]) Fix tag helpers so that all HTML element boolean attributes render according to the specs. Added all boolean attributes listed in the XHTML 1.0 specs ( and HTML 5 specs ( HTML 5 boolean attribute rendering was broken in commit 1e2d7229602f467cfdc0ef606b5ef8a5566a1501 / [#2864 state:resolved].

    Signed-off-by: Yehuda Katz

  • Yehuda Katz (wycats)

    Yehuda Katz (wycats) July 30th, 2009 @ 05:49 PM

    • Milestone cleared.

    @marc the reason this is adequate is that it's only an HTML spec problem and we've included all HTML5 attributes? Seems reasonable to me.

  • Marc Love

    Marc Love July 30th, 2009 @ 08:50 PM

    @wycats was that a question? The phrasing is kind of throwing me off. If you're asking if those are all the HTML5 boolean attributes, yes they are (at least according to the current draft). If you're asking why I included all of them, I just figured I might as well add them all while I had the HTML5 spec in front of me. If you weren't asking a question, then nevermind. :)

  • Yehuda Katz (wycats)

    Yehuda Katz (wycats) July 30th, 2009 @ 09:22 PM

    I was asking about forward compatibility. This isn't an issue because the problem only affected HTML5 attributes and we have them all covered, right?

  • Marc Love

    Marc Love August 1st, 2009 @ 08:20 AM

    Yes, that's correct. All HTML5 boolean attributes are now covered according to the current draft of the HTML5 specs.

  • Ryan Bigg

    Ryan Bigg October 9th, 2010 @ 09:53 PM

    • Tag cleared.

    Automatic cleanup of spam.

  • Ryan Bigg

    Ryan Bigg October 9th, 2010 @ 10:00 PM

    Automatic cleanup of spam.

  • Jeremy Kemper

    Jeremy Kemper October 15th, 2010 @ 11:01 PM

    • Milestone set to 3.0.2
  • Ryan Bigg

    Ryan Bigg November 8th, 2010 @ 01:51 AM

    Automatic cleanup of spam.

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=""></a>

Referenced by