Wicket
  1. Wicket
  2. WICKET-4153

The tbody section of a DataTable is empty when no records are returned by the provider.

    Details

    • Type: Bug Bug
    • Status: Reopened
    • Priority: Trivial Trivial
    • Resolution: Unresolved
    • Affects Version/s: 1.5.1
    • Fix Version/s: None
    • Component/s: wicket-extensions
    • Labels:

      Description

      When a DataTable is rendered without records, the tbody section is empty. This violates the html spec.

      From the spec:
      "When present, each THEAD, TFOOT, and TBODY contains a row group. Each row group must contain at least one row, defined by the TR element."
      and
      "The THEAD, TFOOT, and TBODY sections must contain the same number of columns."

      1. addCenterToolbar.patch
        2 kB
        Sander Plas
      2. DataTable.patch
        16 kB
        Bertrand Guay-Paquet
      3. DataTableTest-usePlainTR.diff
        1 kB
        Sander Plas
      4. DataTable-usePlainTR.diff
        2 kB
        Sander Plas
      5. good.png
        4 kB
        Bertrand Guay-Paquet
      6. with_patch.png
        2 kB
        Bertrand Guay-Paquet
      7. without_patch.png
        2 kB
        Bertrand Guay-Paquet

        Issue Links

          Activity

          Hide
          Martin Grigorov added a comment -

          Suggested workaround from https://bugzilla.mozilla.org/show_bug.cgi?id=409254 :

          tbody:empty

          {display:none;}
          Show
          Martin Grigorov added a comment - Suggested workaround from https://bugzilla.mozilla.org/show_bug.cgi?id=409254 : tbody:empty {display:none;}
          Hide
          Matthias Keller added a comment -

          This bug just bit me too, we do have a filter and we need the table headers etc to be shown but have a message printed below showing that there were no results. Currently it's not possible to style that table correctly (using borders and border-collapse) since it's no valid XHTML and there's a tbody missing. It would be good if this was fixed for the next version!

          Show
          Matthias Keller added a comment - This bug just bit me too, we do have a filter and we need the table headers etc to be shown but have a message printed below showing that there were no results. Currently it's not possible to style that table correctly (using borders and border-collapse) since it's no valid XHTML and there's a tbody missing. It would be good if this was fixed for the next version!
          Hide
          Sander Plas added a comment -

          The empty table could be a result of a filter set by the (end) user. If the whole table is hidden, the filter toolbar is gone too, so the user won't be able to reset/modify the filter.

          Show
          Sander Plas added a comment - The empty table could be a result of a filter set by the (end) user. If the whole table is hidden, the filter toolbar is gone too, so the user won't be able to reset/modify the filter.
          Hide
          Igor Vaynberg added a comment -

          instead of showing an empty table why not hide the table completely and show a no-records message instead?

          Show
          Igor Vaynberg added a comment - instead of showing an empty table why not hide the table completely and show a no-records message instead?
          Hide
          Sander Plas added a comment -

          OK, what about adding the possibility to provide something (a text model? a component?) to DataTable to display instead of the data in case of no records?

          like:

          table.setNoRecordsModel(new Model("no records!"));

          or

          table.setNoRecordsComponent(new Label("no records!"));

          BTW - i don't know what the best way to do this is but i do think that it would be really nice if it would at least be possible for the user to get DataTable to generate valid XHTML1/HTML4.

          Show
          Sander Plas added a comment - OK, what about adding the possibility to provide something (a text model? a component?) to DataTable to display instead of the data in case of no records? like: table.setNoRecordsModel(new Model("no records!")); or table.setNoRecordsComponent(new Label("no records!")); BTW - i don't know what the best way to do this is but i do think that it would be really nice if it would at least be possible for the user to get DataTable to generate valid XHTML1/HTML4.
          Hide
          Igor Vaynberg added a comment -

          this is not a good idea. do the center toolbars go before or after the data? do they show up in every single page of the datatable or only on the first/last page? there are too many implicit assumptions made with this kind of api.

          Show
          Igor Vaynberg added a comment - this is not a good idea. do the center toolbars go before or after the data? do they show up in every single page of the datatable or only on the first/last page? there are too many implicit assumptions made with this kind of api.
          Hide
          Bertrand Guay-Paquet added a comment -

          I know the toolbars don't add themselves (except, in a sense, in DefaultDataTable)

          Your patch is pretty much what I had in mind. In order to not change the current appearance of DefaultDataTable, I would have perhaps preserved the current location (bottom) of the no records toolbar and added another constructor for adding it to the center toolbar.

          Show
          Bertrand Guay-Paquet added a comment - I know the toolbars don't add themselves (except, in a sense, in DefaultDataTable) Your patch is pretty much what I had in mind. In order to not change the current appearance of DefaultDataTable, I would have perhaps preserved the current location (bottom) of the no records toolbar and added another constructor for adding it to the center toolbar.
          Hide
          Sander Plas added a comment -

          @Bertrand: a toolbar does not add itself, but it is explicitly added to the DataTable by the user by calling the addTopToolbar or addBottomToolbar methods on the DataTable.

          So if we want to be able to add toolbars to the tbody, we'll have to modify DataTable.

          It's quite easy to add a third ToolbarContainer besides topToolbars and bottomToolbars (centerToolbars or something) to DataTable, located between the tbody tags.

          I don't know whether this would be considered a desirable change though?

          I have attached a patch for this change to DataTable (+ the call to addCenterToolbar in DefaultDataTable).

          Show
          Sander Plas added a comment - @Bertrand: a toolbar does not add itself, but it is explicitly added to the DataTable by the user by calling the addTopToolbar or addBottomToolbar methods on the DataTable. So if we want to be able to add toolbars to the tbody, we'll have to modify DataTable. It's quite easy to add a third ToolbarContainer besides topToolbars and bottomToolbars (centerToolbars or something) to DataTable, located between the tbody tags. I don't know whether this would be considered a desirable change though? I have attached a patch for this change to DataTable (+ the call to addCenterToolbar in DefaultDataTable).
          Hide
          Sander Plas added a comment -

          Mozilla bug report about the Firefox bug: https://bugzilla.mozilla.org/show_bug.cgi?id=409254

          Show
          Sander Plas added a comment - Mozilla bug report about the Firefox bug: https://bugzilla.mozilla.org/show_bug.cgi?id=409254
          Hide
          Bertrand Guay-Paquet added a comment -

          Yes I understand that and I also agree with Sven's observations. Sorry if I sounded like that wasn't the case.

          However, I still need a way for my tables to look good when they contain no records. I have an ajax filter above the table so without a fix, the user sees the table header borders alternatively switching on and off when no records match the filter. This does not look good at all.

          Instead of changing the behavior of NoRecordsToolbar which would change the behavior of DataTable perhaps too much, how about creating a new type of NoRecordsToolbar that adds itself to the body of the table? This would be optional and not change anything to the current DataTable if not used.

          Show
          Bertrand Guay-Paquet added a comment - Yes I understand that and I also agree with Sven's observations. Sorry if I sounded like that wasn't the case. However, I still need a way for my tables to look good when they contain no records. I have an ajax filter above the table so without a fix, the user sees the table header borders alternatively switching on and off when no records match the filter. This does not look good at all. Instead of changing the behavior of NoRecordsToolbar which would change the behavior of DataTable perhaps too much, how about creating a new type of NoRecordsToolbar that adds itself to the body of the table? This would be optional and not change anything to the current DataTable if not used.
          Hide
          Martin Grigorov added a comment -

          @Bertrand: NoRecordsToolbar is added by default in DefaultDataTable, but it is optional by default. I.e. a user can use DataTable without adding NoRecordsToolbar neither in the top nor in the bottom toolbars.
          Your patch is good Wicket-wise, but as Sven noted it is semantically bad, e.g. it is wrong for SEO.

          Show
          Martin Grigorov added a comment - @Bertrand: NoRecordsToolbar is added by default in DefaultDataTable, but it is optional by default. I.e. a user can use DataTable without adding NoRecordsToolbar neither in the top nor in the bottom toolbars. Your patch is good Wicket-wise, but as Sven noted it is semantically bad, e.g. it is wrong for SEO.
          Hide
          Bertrand Guay-Paquet added a comment -

          @Sander: your patch does produce valid markup. However, when there are no records, the headers become part of the implicit tbody section instead ot thead which can wreck CSS styling.

          @All: It's too bad that my patch is deemed too intrusive. I can't just ignore the rendering bug present in Firefox until this is fixed when an easy workaround that works for me is available. I'll have to keep on using my own version of datatable.

          Would putting the no-records row in the tbody instead of tfoot be acceptable? I very well might be doing such a modification to my DataTable anyway so I could provide a patch.

          Show
          Bertrand Guay-Paquet added a comment - @Sander: your patch does produce valid markup. However, when there are no records, the headers become part of the implicit tbody section instead ot thead which can wreck CSS styling. @All: It's too bad that my patch is deemed too intrusive. I can't just ignore the rendering bug present in Firefox until this is fixed when an easy workaround that works for me is available. I'll have to keep on using my own version of datatable. Would putting the no-records row in the tbody instead of tfoot be acceptable? I very well might be doing such a modification to my DataTable anyway so I could provide a patch.
          Hide
          Sander Plas added a comment -

          @Martin: the idea behind my patch was that it at least gives the user the possibility to generate valid XHTML or HTML4 in case of 0 records by adding a toolbar like NoRecordsToolbar. Just following the HTML5 spec makes this impossible without having to modify/subclass DataTable.

          @All: this doesn't mean that i have very strong feelings about this; any decision on this matter is fine with me

          Show
          Sander Plas added a comment - @Martin: the idea behind my patch was that it at least gives the user the possibility to generate valid XHTML or HTML4 in case of 0 records by adding a toolbar like NoRecordsToolbar. Just following the HTML5 spec makes this impossible without having to modify/subclass DataTable. @All: this doesn't mean that i have very strong feelings about this; any decision on this matter is fine with me
          Hide
          Sven Meier added a comment -

          "I think we should follow HTML5 specs. This is where active development goes."

          +1, my thoughts exactly.

          Show
          Sven Meier added a comment - "I think we should follow HTML5 specs. This is where active development goes." +1, my thoughts exactly.
          Hide
          Martin Grigorov added a comment -

          @Sander: your patches will create <table></table> if there is no data returned from the IDataProvider. This markup is again invalid because <table> should have at least one tbody or tr.

          @All: I think we should follow HTML5 specs. This is where active development goes. Even Firefox will fix their rendering problems with the empty tbody if someone tells them that HTML5 allows this.

          Show
          Martin Grigorov added a comment - @Sander: your patches will create <table></table> if there is no data returned from the IDataProvider. This markup is again invalid because <table> should have at least one tbody or tr. @All: I think we should follow HTML5 specs. This is where active development goes. Even Firefox will fix their rendering problems with the empty tbody if someone tells them that HTML5 allows this.
          Hide
          Sander Plas added a comment -

          Please take a look at my patches - would this be an acceptable solution?

          Show
          Sander Plas added a comment - Please take a look at my patches - would this be an acceptable solution?
          Hide
          Sander Plas added a comment -

          Only show the body of THEAD, TFOOT and TBODY in case of 0 records. Result: TR's added directly to table --> legal (X)HTML as long as at least one toolbar is added.

          Show
          Sander Plas added a comment - Only show the body of THEAD, TFOOT and TBODY in case of 0 records. Result: TR's added directly to table --> legal (X)HTML as long as at least one toolbar is added.
          Hide
          Sander Plas added a comment -

          @Sven
          I've been googling around a bit and it looks like the sentence from the HTML4 document that you quoted earlier:

          "The TBODY start tag is always required except when the table contains only one body and no head or foot sections. The TBODY end tag may always be safely omitted. "

          ... is generally interpreted as:

          "If you don't use THEAD/TFOOT, you can leave out the TBODY too and add the TRs to the TABLE directly."

          This is consistent with XHTML 1 and HTML versions before 4.

          Show
          Sander Plas added a comment - @Sven I've been googling around a bit and it looks like the sentence from the HTML4 document that you quoted earlier: "The TBODY start tag is always required except when the table contains only one body and no head or foot sections. The TBODY end tag may always be safely omitted. " ... is generally interpreted as: "If you don't use THEAD/TFOOT, you can leave out the TBODY too and add the TRs to the TABLE directly." This is consistent with XHTML 1 and HTML versions before 4.
          Hide
          Sander Plas added a comment -

          @Sven
          Strange, i thought XHTML 1.0 was more restrictive than HTML 4? Looks like XHTML allows direct tr children in a table and HTML doesn't:

          http://www.w3.org/TR/xhtml1/dtds.html#a_dtd_XHTML-1.0-Strict (search for tables)

          vs

          http://www.w3.org/TR/html4/struct/tables.html#edef-TABLE

          Show
          Sander Plas added a comment - @Sven Strange, i thought XHTML 1.0 was more restrictive than HTML 4? Looks like XHTML allows direct tr children in a table and HTML doesn't: http://www.w3.org/TR/xhtml1/dtds.html#a_dtd_XHTML-1.0-Strict (search for tables) vs http://www.w3.org/TR/html4/struct/tables.html#edef-TABLE
          Hide
          Bertrand Guay-Paquet added a comment -

          The screenshots shots that the problem is not the border around the whole table, but the border around <thead> and between <th> elements in the <thead> section. Maybe I did not understand your suggestion correctly, but an additional wrapping div would solve all these problems.

          Good point though regarding the javascript looking for tr elements.

          Semantically, IMHO the best we could do is put the "no records" row in tbody instead of tfoot (with proper colspan and maybe css). However, that would be a more involved fix and would bring back the javascript-looking-for-tr problem...

          Show
          Bertrand Guay-Paquet added a comment - The screenshots shots that the problem is not the border around the whole table, but the border around <thead> and between <th> elements in the <thead> section. Maybe I did not understand your suggestion correctly, but an additional wrapping div would solve all these problems. Good point though regarding the javascript looking for tr elements. Semantically, IMHO the best we could do is put the "no records" row in tbody instead of tfoot (with proper colspan and maybe css). However, that would be a more involved fix and would bring back the javascript-looking-for-tr problem...
          Hide
          Sven Meier added a comment -

          @Sander
          The ticket is still open because I reverted my changes, thus we don't have an agreed fix yet. Note that according to the spec a table must have a tbody element and no direct tr children.

          @Bertrand
          IMHO accessibility is the strongest point against an invisble row, it would destroy the semantics of the table.
          Furthermore what about javascript methods that look for tr elements (e.g. a drag and drop solution)?

          w3c probably noticed their mistake in making a tr inside tbody mandatory and fixed their decision in HTML5. Firefox' border-bug can easily be bypassed with an additional wrapping div. That's what the CSS designers do all the time anyway.

          That's my take on this issue, I don't approve your patch. You may convince another dev to follow your arguments.

          Thanks for your dedication on this issue.

          Show
          Sven Meier added a comment - @Sander The ticket is still open because I reverted my changes, thus we don't have an agreed fix yet. Note that according to the spec a table must have a tbody element and no direct tr children. @Bertrand IMHO accessibility is the strongest point against an invisble row, it would destroy the semantics of the table. Furthermore what about javascript methods that look for tr elements (e.g. a drag and drop solution)? w3c probably noticed their mistake in making a tr inside tbody mandatory and fixed their decision in HTML5. Firefox' border-bug can easily be bypassed with an additional wrapping div. That's what the CSS designers do all the time anyway. That's my take on this issue, I don't approve your patch. You may convince another dev to follow your arguments. Thanks for your dedication on this issue.
          Hide
          Bertrand Guay-Paquet added a comment -

          Please see the attached screenshots to see the problem. The borders are not rendered properly with an empty tbody section.

          I am not particularly fond of the bogus row made invisible with CSS either. However, it does produce valid HTML. Adding a no-records row in tbody instead of tfoot would be a better solution. Would there still be problems with that solution? It would of course make the no-records toolbar mandatory to produce valid markup.

          @Sven: I disagree with your comment about HTML5. It is not even a finished standard yet and it is definitely not widely supported by browsers yet. Ignoring the HTML4 problem because HTML5 relaxes the rules is not acceptable. Firefox renders HTML5 and it fails to render tables properly with an empty tbody.

          @Sander: In theory, browsers can repeat thead and tfoot sections for long tbody sections. I believe it also provides better context for screen readers as well.

          Show
          Bertrand Guay-Paquet added a comment - Please see the attached screenshots to see the problem. The borders are not rendered properly with an empty tbody section. I am not particularly fond of the bogus row made invisible with CSS either. However, it does produce valid HTML. Adding a no-records row in tbody instead of tfoot would be a better solution. Would there still be problems with that solution? It would of course make the no-records toolbar mandatory to produce valid markup. @Sven: I disagree with your comment about HTML5. It is not even a finished standard yet and it is definitely not widely supported by browsers yet. Ignoring the HTML4 problem because HTML5 relaxes the rules is not acceptable. Firefox renders HTML5 and it fails to render tables properly with an empty tbody. @Sander: In theory, browsers can repeat thead and tfoot sections for long tbody sections. I believe it also provides better context for screen readers as well.
          Hide
          Bertrand Guay-Paquet added a comment -

          Table screenshots showing problem in Firefox

          Show
          Bertrand Guay-Paquet added a comment - Table screenshots showing problem in Firefox
          Hide
          Sander Plas added a comment -

          What's exactly the advantage of using THEAD/TFOOT/TBODY instead of just TR's? Is it just layout/css related?

          Anyway, i just wanted to add that using plain TR's for everyting would probably solve this problem, as long as at least one toolbar is displayed.

          Sven; i see that the ticket is still open, is that intentional?

          If this is the final decision on this matter, i'm going to ask dashorst to add this one to wicketstuff htmlvalidator's "ignore known wicket bugs" filter.

          Show
          Sander Plas added a comment - What's exactly the advantage of using THEAD/TFOOT/TBODY instead of just TR's? Is it just layout/css related? Anyway, i just wanted to add that using plain TR's for everyting would probably solve this problem, as long as at least one toolbar is displayed. Sven; i see that the ticket is still open, is that intentional? If this is the final decision on this matter, i'm going to ask dashorst to add this one to wicketstuff htmlvalidator's "ignore known wicket bugs" filter.
          Hide
          Sven Meier added a comment -

          Thanks Sander for the hint: If HTML5 allows tbody without tr, we should just keep DataTable as it worked before.

          Reusing tbody for header or footer might get too complicated.

          Show
          Sven Meier added a comment - Thanks Sander for the hint: If HTML5 allows tbody without tr, we should just keep DataTable as it worked before. Reusing tbody for header or footer might get too complicated.
          Hide
          Sander Plas added a comment -

          Can't we turn this TR in something useful instead of making it invisible? For example, put the toolbars (headers, NoRecordsToolbar, etc.) to the TBODY instead of THEAD/TFOOT in case of 0 records?

          BTW, HTML5 seems to allow empty TBODY's: http://www.w3.org/TR/html5/tabular-data.html#the-tbody-element

          Show
          Sander Plas added a comment - Can't we turn this TR in something useful instead of making it invisible? For example, put the toolbars (headers, NoRecordsToolbar, etc.) to the TBODY instead of THEAD/TFOOT in case of 0 records? BTW, HTML5 seems to allow empty TBODY's: http://www.w3.org/TR/html5/tabular-data.html#the-tbody-element
          Hide
          Sven Meier added a comment -

          Well, semantically this is a no-go. What should a screen-reader do with such an empty <tr>?

          Bertrand, could we see an example of a failing border decoration?

          Show
          Sven Meier added a comment - Well, semantically this is a no-go. What should a screen-reader do with such an empty <tr>? Bertrand, could we see an example of a failing border decoration?
          Hide
          Martin Grigorov added a comment -

          According to Bertrand, Firefox breaks the table borders with empty tbody.
          http://www.w3.org/TR/xhtml1/dtds.html#a_dtd_XHTML-1.0-Strict says that <table> should have either at least one tbody or tr. If it is tbody then it has to have at least one tr.
          http://www.w3.org/TR/html4/struct/tables.html#h-11.2.1 also mandates the same.

          I kinda like the attached patch because I don't see another solution.

          Show
          Martin Grigorov added a comment - According to Bertrand, Firefox breaks the table borders with empty tbody. http://www.w3.org/TR/xhtml1/dtds.html#a_dtd_XHTML-1.0-Strict says that <table> should have either at least one tbody or tr. If it is tbody then it has to have at least one tr. http://www.w3.org/TR/html4/struct/tables.html#h-11.2.1 also mandates the same. I kinda like the attached patch because I don't see another solution.
          Hide
          Sven Meier added a comment -

          still undecided

          Show
          Sven Meier added a comment - still undecided
          Hide
          Sven Meier added a comment -

          Ah yes, I always mix up these two.

          It seems we're in a quandary here. I really don't like an invisible styled tr suggested by Bertrand.

          Note that the spec sounds pretty lenient, e.g. (http://www.w3.org/TR/html4/struct/tables.html#edef-TBODY) "The TBODY start tag is always required except when the table contains only one table body and no table head or foot sections." Perhaps it's better just to leave the tbody as it was (always there) even if this means a small inconsistency with the spec.

          Show
          Sven Meier added a comment - Ah yes, I always mix up these two. It seems we're in a quandary here. I really don't like an invisible styled tr suggested by Bertrand. Note that the spec sounds pretty lenient, e.g. ( http://www.w3.org/TR/html4/struct/tables.html#edef-TBODY ) "The TBODY start tag is always required except when the table contains only one table body and no table head or foot sections." Perhaps it's better just to leave the tbody as it was (always there) even if this means a small inconsistency with the spec.
          Hide
          Martin Grigorov added a comment -

          It is actually onConfigure(), not onInitialize(). But the question is: is the generated html XHTML valid ? See WICKET-4142 - it says that a <table> should have <tbody> or <tr>, while with the current code the generated markup can look like: <table></table>.

          Show
          Martin Grigorov added a comment - It is actually onConfigure(), not onInitialize(). But the question is: is the generated html XHTML valid ? See WICKET-4142 - it says that a <table> should have <tbody> or <tr>, while with the current code the generated markup can look like: <table></table>.
          Hide
          Sven Meier added a comment -

          visibility is now checked in onInitialize

          Show
          Sven Meier added a comment - visibility is now checked in onInitialize
          Hide
          Bertrand Guay-Paquet added a comment -

          Sorry, I missed the other issue.

          Also wanted to mention that this is not strictly for validation purposes. Firefox does not render table borders correctly with an empty tbody section.

          Show
          Bertrand Guay-Paquet added a comment - Sorry, I missed the other issue. Also wanted to mention that this is not strictly for validation purposes. Firefox does not render table borders correctly with an empty tbody section.
          Hide
          Bertrand Guay-Paquet added a comment -

          This patch has a test case and solves this problem by adding an empty row to the tbody and hiding it with CSS styling. It also removes some DataTable related java warnings.

          Show
          Bertrand Guay-Paquet added a comment - This patch has a test case and solves this problem by adding an empty row to the tbody and hiding it with CSS styling. It also removes some DataTable related java warnings.

            People

            • Assignee:
              Unassigned
              Reporter:
              Bertrand Guay-Paquet
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:

                Development