Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
If I enable a vocabulary the terms are shown in invalid HTML code. There is something wrong the the UL > LI's. Need to investigate further... it also have the "tree" class that should be themable.
Comment | File | Size | Author |
---|---|---|---|
#14 | site_map_652478-14.patch | 8.68 KB | tic2000 |
Comments
Comment #1
frjo CreditAttribution: frjo commentedThere used to be some small validation errors with the ul nesting but they should have been fixed.
Is't the "tree" in
<div class="tree">
you mean? That div locks redundant to me, probably something left over from older versions. Just committed a patch that removes that div and makes some followup changes to the CSS file.
Please try it out and report back here.
Comment #2
hass CreditAttribution: hass commentedThat great - thank you.
The DIV looked to be redundant to me, too. Are you able to give the UL a class name so that themers are able to style the it (like we have done for the menu)? I tried to understand the taxonomy tree building code yesterday and thought it may be a good idea to make it easier to understand, but I was holded back with other things... I have simply not understood without digging deeper what you are doing there... maybe there are theme functions that can be reused!?
Comment #3
hass CreditAttribution: hass commentedThere is no commit that changes the UL?
Comment #4
frjo CreditAttribution: frjo commentedThe nested UL validation problem was fixed some time ago.
A clss for the UL is not needed I think. Use the surrounding div with classes like "sitemap-terms sitemap-terms-[vid]" instead.
Comment #5
hass CreditAttribution: hass commentedI used latest dev and the bug was inside and not fixed... Need to retest.
Comment #6
hass CreditAttribution: hass commentedVerified again, but is not fixed in latest DEV
Comment #7
frjo CreditAttribution: frjo commentedCan you put a link to this site map or post a chuck of the HTML?
I tested my own site map pages and found no problems with them.
Comment #8
hass CreditAttribution: hass commentedFirst of all a vocabulary without any terms. Does it makes sense to show it? I think this should only outputted of it have terms.
Vocabulary with 3 levels
Error:
Comment #9
frjo CreditAttribution: frjo commentedThat snippet of HTML looks valid (XHTML 1.0 Strict) to me and when I check it on http://validator.w3.org/ it reports no problems.
Comment #10
hass CreditAttribution: hass commentedMaybe Firebug changed something... here is the raw copy from Firefox source code... W3C validator also shows me the errors.
Comment #11
frjo CreditAttribution: frjo commentedYes, now I see the errors also.
This is tricky, I will try to fix this bug but any help it appreciated.
Comment #12
hass CreditAttribution: hass commentedIt may be a good odea to refactor this function and if possible - reuse some core functions or at least copy them over as a base... It looked a bit difficult to me too…
Comment #13
hass CreditAttribution: hass commentedMarked #747022: html problem on menus as duplicate
Comment #14
tic2000 CreditAttribution: tic2000 commentedComment #15
tic2000 CreditAttribution: tic2000 commentedPatch still applies after #730992: Options for title of sitemap page was committed.
Comment #16
tic2000 CreditAttribution: tic2000 commentedNo review after more than 5 weeks?
Comment #17
frjo CreditAttribution: frjo commentedSorry tic2000, has been a busy spring summer. It's on my todo list, I promise.
Comment #18
frjo CreditAttribution: frjo commentedThis patch needs to be rerolled against latest 6--2-dev.
There seems also to be a conflict between the use of $class in this patch and the old use of $class in theme_site_map_box().