Issue status update for http://drupal.org/node/30423 Post a follow up: http://drupal.org/project/comments/add/30423 Project: Drupal Version: cvs Component: theme system Category: feature requests Priority: normal Assigned to: Anonymous Reported by: Crell Updated by: Kobus Status: patch (code needs review) Hi, I won't comment on the implementation, because I don't know the impact of this, but from a usability pespective it is a HUGE +1 for me, as some screen readers use either the or tag to describe table contents to blind users. An example of a browser/screen reader integration that uses the tag is IBM's Homepage Reader. Regards, Kobus Kobus Previous comments: ------------------------------------------------------------------------ Mon, 05 Sep 2005 07:12:18 +0000 : Crell Attachment: http://drupal.org/files/issues/theme_table_0.patch (1001 bytes) Well, if I made this patch properly :-), this patch adds a fourth parameter to the theme_table() function, $caption. If set, it adds a <caption>$caption</caption> line to the table. If it is not set, this patch has no effect. All styling is left up to CSS to figure out. It seems to me that $caption would make more sense as the 3rd parameter, before $attributes, but that would break backward compatibility so I just tacked it on at the end. I'll leave the final order of parameters up to someone else to figure out.

Issue status update for http://drupal.org/node/30423 Post a follow up: http://drupal.org/project/comments/add/30423 Project: Drupal Version: cvs Component: theme system Category: feature requests Priority: normal Assigned to: Anonymous Reported by: Crell Updated by: Crell Status: patch (code needs review) Well, summary is an attribute of <table>, not an element in itself. You can already set that with the $attributes parameter. I don't know if the Powers That Be would want another parameter just for that attribute, although that might increase its usage. I just needed the caption because I have two tables on one admin page, and not having some way to tell them apart was not good. Crell Previous comments: ------------------------------------------------------------------------ Mon, 05 Sep 2005 07:12:18 +0000 : Crell Attachment: http://drupal.org/files/issues/theme_table_0.patch (1001 bytes) Well, if I made this patch properly :-), this patch adds a fourth parameter to the theme_table() function, $caption. If set, it adds a <caption>$caption</caption> line to the table. If it is not set, this patch has no effect. All styling is left up to CSS to figure out. It seems to me that $caption would make more sense as the 3rd parameter, before $attributes, but that would break backward compatibility so I just tacked it on at the end. I'll leave the final order of parameters up to someone else to figure out. ------------------------------------------------------------------------ Mon, 05 Sep 2005 07:19:10 +0000 : Kobus Hi, I won't comment on the implementation, because I don't know the impact of this, but from a usability pespective it is a HUGE +1 for me, as some screen readers use either the or tag to describe table contents to blind users. An example of a browser/screen reader integration that uses the tag is IBM's Homepage Reader. Regards, Kobus ------------------------------------------------------------------------ Mon, 05 Sep 2005 07:20:23 +0000 : Kobus Hi, I won't comment on the implementation, because I don't know the impact of this, but from a usability pespective it is a HUGE +1 for me, as some screen readers use either the <summary> or <caption> tag to describe table contents to blind users. An example of a browser/screen reader integration that uses the <summary> tag is IBM's Homepage Reader. Regards, Kobus

Issue status update for http://drupal.org/node/30423 Post a follow up: http://drupal.org/project/comments/add/30423 Project: Drupal Version: cvs Component: theme system Category: feature requests Priority: normal Assigned to: Anonymous Reported by: Crell Updated by: Kobus Status: patch (code needs review) Yes, indeed, you are correct. Pardon my oversight that it is already a attribute. I evaluated homepage reader about 2 months ago, and merely recalled summary. Sorry! Kobus Kobus Previous comments: ------------------------------------------------------------------------ Mon, 05 Sep 2005 07:12:18 +0000 : Crell Attachment: http://drupal.org/files/issues/theme_table_0.patch (1001 bytes) Well, if I made this patch properly :-), this patch adds a fourth parameter to the theme_table() function, $caption. If set, it adds a <caption>$caption</caption> line to the table. If it is not set, this patch has no effect. All styling is left up to CSS to figure out. It seems to me that $caption would make more sense as the 3rd parameter, before $attributes, but that would break backward compatibility so I just tacked it on at the end. I'll leave the final order of parameters up to someone else to figure out. ------------------------------------------------------------------------ Mon, 05 Sep 2005 07:19:10 +0000 : Kobus Hi, I won't comment on the implementation, because I don't know the impact of this, but from a usability pespective it is a HUGE +1 for me, as some screen readers use either the or tag to describe table contents to blind users. An example of a browser/screen reader integration that uses the tag is IBM's Homepage Reader. Regards, Kobus ------------------------------------------------------------------------ Mon, 05 Sep 2005 07:20:23 +0000 : Kobus Hi, I won't comment on the implementation, because I don't know the impact of this, but from a usability pespective it is a HUGE +1 for me, as some screen readers use either the <summary> or <caption> tag to describe table contents to blind users. An example of a browser/screen reader integration that uses the <summary> tag is IBM's Homepage Reader. Regards, Kobus ------------------------------------------------------------------------ Mon, 05 Sep 2005 07:45:29 +0000 : Crell Well, summary is an attribute of <table>, not an element in itself. You can already set that with the $attributes parameter. I don't know if the Powers That Be would want another parameter just for that attribute, although that might increase its usage. I just needed the caption because I have two tables on one admin page, and not having some way to tell them apart was not good. ------------------------------------------------------------------------ Mon, 05 Sep 2005 08:20:54 +0000 : Bèr Kessels +1 from me too. This is a huge accessability improvement, yet a very small patch. We should follow this patch up, with a list of theme('table') calls that should use this caption.

Issue status update for http://drupal.org/node/30423 Post a follow up: http://drupal.org/project/comments/add/30423 Project: Drupal Version: cvs Component: theme system Category: feature requests Priority: normal Assigned to: Anonymous Reported by: Crell Updated by: Robrecht Jacques -Status: patch (code needs review) +Status: patch (code needs work) No Bèr; "summary" is an attribute of "table" and is already supported (although nowhere used) by the "$attributes" argument. "caption" is NO attribute of "table" but an element by itself, eg it can contain HTML code (which attributes can't contain). So: +1 for this, but the core module theme('table') uses should be checked to see if we can add it somewhere. So: code needs work: go through the uses in core and add a caption where it would be usefull. Robrecht Jacques Previous comments: ------------------------------------------------------------------------ Mon, 05 Sep 2005 07:12:18 +0000 : Crell Attachment: http://drupal.org/files/issues/theme_table_0.patch (1001 bytes) Well, if I made this patch properly :-), this patch adds a fourth parameter to the theme_table() function, $caption. If set, it adds a <caption>$caption</caption> line to the table. If it is not set, this patch has no effect. All styling is left up to CSS to figure out. It seems to me that $caption would make more sense as the 3rd parameter, before $attributes, but that would break backward compatibility so I just tacked it on at the end. I'll leave the final order of parameters up to someone else to figure out. ------------------------------------------------------------------------ Mon, 05 Sep 2005 07:19:10 +0000 : Kobus Hi, I won't comment on the implementation, because I don't know the impact of this, but from a usability pespective it is a HUGE +1 for me, as some screen readers use either the or tag to describe table contents to blind users. An example of a browser/screen reader integration that uses the tag is IBM's Homepage Reader. Regards, Kobus ------------------------------------------------------------------------ Mon, 05 Sep 2005 07:20:23 +0000 : Kobus Hi, I won't comment on the implementation, because I don't know the impact of this, but from a usability pespective it is a HUGE +1 for me, as some screen readers use either the <summary> or <caption> tag to describe table contents to blind users. An example of a browser/screen reader integration that uses the <summary> tag is IBM's Homepage Reader. Regards, Kobus ------------------------------------------------------------------------ Mon, 05 Sep 2005 07:45:29 +0000 : Crell Well, summary is an attribute of <table>, not an element in itself. You can already set that with the $attributes parameter. I don't know if the Powers That Be would want another parameter just for that attribute, although that might increase its usage. I just needed the caption because I have two tables on one admin page, and not having some way to tell them apart was not good. ------------------------------------------------------------------------ Mon, 05 Sep 2005 08:20:54 +0000 : Bèr Kessels +1 from me too. This is a huge accessability improvement, yet a very small patch. We should follow this patch up, with a list of theme('table') calls that should use this caption. ------------------------------------------------------------------------ Mon, 05 Sep 2005 08:22:50 +0000 : Kobus Yes, indeed, you are correct. Pardon my oversight that it is already a attribute. I evaluated homepage reader about 2 months ago, and merely recalled summary. Sorry! Kobus ------------------------------------------------------------------------ Mon, 05 Sep 2005 10:05:41 +0000 : Bèr Kessels I must follow Kobus. Excuse my ignorance in this too. This should indeed be set with attributes, which already is perfectly possible. Unless someone comes with new arguments, I suggest we close this issue as "won't fix".

Issue status update for http://drupal.org/node/30423 Post a follow up: http://drupal.org/project/comments/add/30423 Project: Drupal Version: cvs Component: theme system Category: feature requests Priority: normal Assigned to: Anonymous Reported by: Crell Updated by: Crell Status: patch (code needs work) OK, to clarify: - caption is a sub-element of <table>. Prior to this patch, there is no way to set it at all. - summary is an attribute of <table>. It can be set via the $attributes parameter now. This patch does not do anything with regards to summary. I can see about suggesting caption locations in core, but I believe that should be a separate patch from adding the caption support in the first place. I'd hate to see captions not available to module developers when 4.7 freezes in a few days just because we couldn't agree on where there should be captions in core. :-) Should I make a separate patch and post it in this thread, or as a new thread? Crell Previous comments: ------------------------------------------------------------------------ Mon, 05 Sep 2005 07:12:18 +0000 : Crell Attachment: http://drupal.org/files/issues/theme_table_0.patch (1001 bytes) Well, if I made this patch properly :-), this patch adds a fourth parameter to the theme_table() function, $caption. If set, it adds a <caption>$caption</caption> line to the table. If it is not set, this patch has no effect. All styling is left up to CSS to figure out. It seems to me that $caption would make more sense as the 3rd parameter, before $attributes, but that would break backward compatibility so I just tacked it on at the end. I'll leave the final order of parameters up to someone else to figure out. ------------------------------------------------------------------------ Mon, 05 Sep 2005 07:19:10 +0000 : Kobus Hi, I won't comment on the implementation, because I don't know the impact of this, but from a usability pespective it is a HUGE +1 for me, as some screen readers use either the or tag to describe table contents to blind users. An example of a browser/screen reader integration that uses the tag is IBM's Homepage Reader. Regards, Kobus ------------------------------------------------------------------------ Mon, 05 Sep 2005 07:20:23 +0000 : Kobus Hi, I won't comment on the implementation, because I don't know the impact of this, but from a usability pespective it is a HUGE +1 for me, as some screen readers use either the <summary> or <caption> tag to describe table contents to blind users. An example of a browser/screen reader integration that uses the <summary> tag is IBM's Homepage Reader. Regards, Kobus ------------------------------------------------------------------------ Mon, 05 Sep 2005 07:45:29 +0000 : Crell Well, summary is an attribute of <table>, not an element in itself. You can already set that with the $attributes parameter. I don't know if the Powers That Be would want another parameter just for that attribute, although that might increase its usage. I just needed the caption because I have two tables on one admin page, and not having some way to tell them apart was not good. ------------------------------------------------------------------------ Mon, 05 Sep 2005 08:20:54 +0000 : Bèr Kessels +1 from me too. This is a huge accessability improvement, yet a very small patch. We should follow this patch up, with a list of theme('table') calls that should use this caption. ------------------------------------------------------------------------ Mon, 05 Sep 2005 08:22:50 +0000 : Kobus Yes, indeed, you are correct. Pardon my oversight that it is already a attribute. I evaluated homepage reader about 2 months ago, and merely recalled summary. Sorry! Kobus ------------------------------------------------------------------------ Mon, 05 Sep 2005 10:05:41 +0000 : Bèr Kessels I must follow Kobus. Excuse my ignorance in this too. This should indeed be set with attributes, which already is perfectly possible. Unless someone comes with new arguments, I suggest we close this issue as "won't fix". ------------------------------------------------------------------------ Mon, 05 Sep 2005 11:49:10 +0000 : Robrecht Jacques No Bèr; "summary" is an attribute of "table" and is already supported (although nowhere used) by the "$attributes" argument. "caption" is NO attribute of "table" but an element by itself, eg it can contain HTML code (which attributes can't contain). So: +1 for this, but the core module theme('table') uses should be checked to see if we can add it somewhere. So: code needs work: go through the uses in core and add a caption where it would be usefull. ------------------------------------------------------------------------ Mon, 05 Sep 2005 11:56:05 +0000 : Kobus I don't think that <caption> is an attribute, only /summary/. Therefore, the patch stands as is; my first note about code> was wrong. The patch is therefore necessary, but ONLY for <caption> and not for summary. Kobus

Issue status update for http://drupal.org/node/30423 Post a follow up: http://drupal.org/project/comments/add/30423 Project: Drupal Version: cvs Component: theme system Category: feature requests Priority: normal Assigned to: Anonymous Reported by: Crell Updated by: Robrecht Jacques -Status: patch (code needs work) +Status: patch (ready to be committed) OK. Patch is ready for commit then: applies, conforms to coding style, works as advertised, has no side effects. Robrecht Jacques Previous comments: ------------------------------------------------------------------------ Mon, 05 Sep 2005 07:12:18 +0000 : Crell Attachment: http://drupal.org/files/issues/theme_table_0.patch (1001 bytes) Well, if I made this patch properly :-), this patch adds a fourth parameter to the theme_table() function, $caption. If set, it adds a <caption>$caption</caption> line to the table. If it is not set, this patch has no effect. All styling is left up to CSS to figure out. It seems to me that $caption would make more sense as the 3rd parameter, before $attributes, but that would break backward compatibility so I just tacked it on at the end. I'll leave the final order of parameters up to someone else to figure out. ------------------------------------------------------------------------ Mon, 05 Sep 2005 07:19:10 +0000 : Kobus Hi, I won't comment on the implementation, because I don't know the impact of this, but from a usability pespective it is a HUGE +1 for me, as some screen readers use either the or tag to describe table contents to blind users. An example of a browser/screen reader integration that uses the tag is IBM's Homepage Reader. Regards, Kobus ------------------------------------------------------------------------ Mon, 05 Sep 2005 07:20:23 +0000 : Kobus Hi, I won't comment on the implementation, because I don't know the impact of this, but from a usability pespective it is a HUGE +1 for me, as some screen readers use either the <summary> or <caption> tag to describe table contents to blind users. An example of a browser/screen reader integration that uses the <summary> tag is IBM's Homepage Reader. Regards, Kobus ------------------------------------------------------------------------ Mon, 05 Sep 2005 07:45:29 +0000 : Crell Well, summary is an attribute of <table>, not an element in itself. You can already set that with the $attributes parameter. I don't know if the Powers That Be would want another parameter just for that attribute, although that might increase its usage. I just needed the caption because I have two tables on one admin page, and not having some way to tell them apart was not good. ------------------------------------------------------------------------ Mon, 05 Sep 2005 08:20:54 +0000 : Bèr Kessels +1 from me too. This is a huge accessability improvement, yet a very small patch. We should follow this patch up, with a list of theme('table') calls that should use this caption. ------------------------------------------------------------------------ Mon, 05 Sep 2005 08:22:50 +0000 : Kobus Yes, indeed, you are correct. Pardon my oversight that it is already a attribute. I evaluated homepage reader about 2 months ago, and merely recalled summary. Sorry! Kobus ------------------------------------------------------------------------ Mon, 05 Sep 2005 10:05:41 +0000 : Bèr Kessels I must follow Kobus. Excuse my ignorance in this too. This should indeed be set with attributes, which already is perfectly possible. Unless someone comes with new arguments, I suggest we close this issue as "won't fix". ------------------------------------------------------------------------ Mon, 05 Sep 2005 11:49:10 +0000 : Robrecht Jacques No Bèr; "summary" is an attribute of "table" and is already supported (although nowhere used) by the "$attributes" argument. "caption" is NO attribute of "table" but an element by itself, eg it can contain HTML code (which attributes can't contain). So: +1 for this, but the core module theme('table') uses should be checked to see if we can add it somewhere. So: code needs work: go through the uses in core and add a caption where it would be usefull. ------------------------------------------------------------------------ Mon, 05 Sep 2005 11:56:05 +0000 : Kobus I don't think that <caption> is an attribute, only /summary/. Therefore, the patch stands as is; my first note about code> was wrong. The patch is therefore necessary, but ONLY for <caption> and not for summary. Kobus ------------------------------------------------------------------------ Mon, 05 Sep 2005 20:05:03 +0000 : Crell OK, to clarify: - caption is a sub-element of <table>. Prior to this patch, there is no way to set it at all. - summary is an attribute of <table>. It can be set via the $attributes parameter now. This patch does not do anything with regards to summary. I can see about suggesting caption locations in core, but I believe that should be a separate patch from adding the caption support in the first place. I'd hate to see captions not available to module developers when 4.7 freezes in a few days just because we couldn't agree on where there should be captions in core. :-) Should I make a separate patch and post it in this thread, or as a new thread? ------------------------------------------------------------------------ Tue, 06 Sep 2005 02:46:30 +0000 : thehunmonkgroup i agree w/ Crell here--use cases in core are a seperate issue from the functionality itself. patch looks good, so let's get this in and make another patch to clean that up.