Bug 12863 - News in OPAC: HTML broken with nested p tags and multiple thead in same table
Summary: News in OPAC: HTML broken with nested p tags and multiple thead in same table
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P4 minor (vote)
Assignee: Marc Véron
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-09-03 08:32 UTC by Marc Véron
Modified: 2015-06-04 23:33 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 12863 - News in OPAC: HTML broken with nested p tags and multiple thead in same table (2.19 KB, patch)
2014-09-03 10:24 UTC, Marc Véron
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 12863 - News in OPAC: HTML broken with nested p tags and multiple thead in same table (2.29 KB, patch)
2014-09-05 13:09 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 12863 [LESS follow-up] News in OPAC: HTML broken with nested p tags and multiple thead in same table (1.32 KB, patch)
2014-09-05 13:10 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 12863 [CSS follow-up] News in OPAC: HTML broken with nested p tags and multiple thead in same table (84.12 KB, patch)
2014-09-05 13:10 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 12863 [LESS follow-up] News in OPAC: HTML broken with nested p tags and multiple thead in same table (1.42 KB, patch)
2014-09-05 13:30 UTC, Marc Véron
Details | Diff | Splinter Review
[Signed-off] Bug 12863 [CSS follow-up] News in OPAC: HTML broken with nested p tags and multiple thead in same table (84.29 KB, patch)
2014-09-05 13:33 UTC, Marc Véron
Details | Diff | Splinter Review
[PASSED QA] Bug 12863 - News in OPAC: HTML broken with nested p tags and multiple thead in same table (2.35 KB, patch)
2014-09-07 22:17 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 12863 [LESS follow-up] News in OPAC: HTML broken with nested p tags and multiple thead in same table (1.48 KB, patch)
2014-09-07 22:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 12863 [CSS follow-up] News in OPAC: HTML broken with nested p tags and multiple thead in same table (84.34 KB, patch)
2014-09-07 22:18 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marc Véron 2014-09-03 08:32:46 UTC
HTML validator displays errors for news in OPAC. 
I used HTMML validator 0.9.5.8 for Firefox.

1)
nested p-Tag like 
 <tbody><tr><td><p><p>This is the text...</p></p> (...)
 This is because opac-main.tt encloses the news text with a p tag. This is not necessary since the News editor delivers HTML formatted text.

To test: 
Create news with just on line. Display in OPAC and test with HTML validator or check page source code. 

2)
multiple thead in same table if you have more than one news

To test:
Create multiple news for OPAC. HTML validator will complain about multiple thead tags in same table. 
This is not allowed, see: www.w3.org/TR/html-markup/table.html#table
Comment 1 Marc Véron 2014-09-03 10:24:31 UTC Comment hidden (obsolete)
Comment 2 Marc Véron 2014-09-03 10:29:30 UTC
Setting to P4 since HTML is broken and accessibiliy by screen readers affected.
Comment 3 Owen Leonard 2014-09-05 13:09:31 UTC Comment hidden (obsolete)
Comment 4 Owen Leonard 2014-09-05 13:10:46 UTC Comment hidden (obsolete)
Comment 5 Owen Leonard 2014-09-05 13:10:52 UTC Comment hidden (obsolete)
Comment 6 Marc Véron 2014-09-05 13:30:28 UTC Comment hidden (obsolete)
Comment 7 Marc Véron 2014-09-05 13:33:55 UTC Comment hidden (obsolete)
Comment 8 Katrin Fischer 2014-09-07 22:17:51 UTC
Created attachment 31443 [details] [review]
[PASSED QA] Bug 12863 - News in OPAC: HTML broken with nested p tags and multiple thead in same table

This patch fixes broken HTML in OPAC news by replacing table display
with divs.

To test:

Create multiple news for OPAC
Validate with HTML validator or check source code
Result:
Error with nested p tags
Table header thead is repeated for every news (not allowed)
Bonus testing: Let a screen reader like NVDA read the news and try to
understand it with closed eyes.

Apply patch
Result:
No more HTML errors in news
Screen reader reads news properly

To style you can use classes newscontainer, newsitem, newsheader,
newsbody and newsfooter.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described, passes tests and QA script.
Comment 9 Katrin Fischer 2014-09-07 22:18:03 UTC
Created attachment 31444 [details] [review]
[PASSED QA] Bug 12863 [LESS follow-up] News in OPAC: HTML broken with nested p tags and multiple thead in same table

This follow-up modifies the LESS file to add style to the new markup to
match the previous markup. A subsequent patch will include the revised
and compiled CSS.

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 10 Katrin Fischer 2014-09-07 22:18:22 UTC
Created attachment 31445 [details] [review]
[PASSED QA] Bug 12863 [CSS follow-up] News in OPAC: HTML broken with nested p tags and multiple thead in same table

This patch contains the revised and compiled CSS which will style the
OPAC's new news markup to match the previous version.

To test, apply the patch and clear your browser cache if necessary. View
news in the OPAC and confirm that it is styled like it was before the
patch making markup changes.

CSS patch behaves as expected. Thanks to Owen for adding this.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 11 Tomás Cohen Arazi 2014-09-18 00:59:42 UTC
Patches pushed to master.

Thanks Marc!