Closed (fixed)
Project:
Chaos Tool Suite (ctools)
Version:
6.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
30 May 2009 at 14:03 UTC
Updated:
3 Aug 2009 at 17:00 UTC
Jump to comment: Most recent file
Comments
Comment #1
dkruglyak commentedComment #2
nickl commentedFrom W3C on css Case:
http://www.w3.org/TR/css3-selectors/#casesens
All Selectors syntax is case-insensitive within the ASCII range (i.e. [a-z] and [A-Z] are equivalent), except for parts that are not under the control of Selectors. The case sensitivity of document language element names, attribute names, and attribute values in selectors depends on the document language. For example, in HTML, element names are case-insensitive, but in XML, they are case-sensitive.
As from the xhtml standard all html mark-up should also be all lower case now and elements the don't have closing tags should close their tags. i.o.w. the
you added should be lowercase and also close the tag
We add a space between the
for backward compatibility with older browsers.
I am leaning towards not fixing this.
Comment #3
dkruglyak commentedXHTML standard is great, but unfortunately in the real world we often have to work with themes created by someone else that do not follow all guidelines.
I have just such a case and find it impossible to do theming even on the newest of the new browsers (Chrome 2.0 and FF 3.0). I do not think CTools module should force the standard on projects that are not able to fully support it and would have to go broken instead.
IF an implementer DOES care about XHTML, they will not use capitalization to begin with. Please fix this problem or at least make lowercase optional!
Comment #4
nickl commentedTidy will help convert the faulty markup for you or use the online HTML Converter.
I still suggest enforce and follow w3c standards.
Comment #5
dkruglyak commented@nickl: What good is the standard if you have to work with broken 3rd party code? This is a major defect of the module right now.
Please make XHTML compliance optional. Thank you!
Comment #6
nickl commented@dkruglyak Unfortunately the choice is not up to me, I'm merely placing a -1 here against breaking xhtml standards.
You can do a lot worse than rolling the patch again without the sequence of
tags, which will guarantee a frowning upon, if you really want this patch to be considered.
Good luck!
Comment #7
dkruglyak commentedI hope whoever reviews this patch would be able to understand it. It should not be that hard.
Trying to force XHTML on everyone is a bad idea.
Comment #8
merlinofchaos commentedUhh.
What's with the brbrbrbrbrbr? I can understand the case sensitivity issue and I'm not actually opposed to it, but the patch seems like it's got some unexplained cruft in it.
Comment #9
dkruglyak commentedPlease ignore "brbrbrbrbrbr" chunk! Delegator UI had problems with some themes causing the task popup be obscured by page admin box boundary. Adding spacing via line breaks was a quick hack to expand the box (I was not sure how to fix CSS), but now the Page Manager obviates the need for this fix.
Aside from that, case sensitivity piece works perfect for me. Marking RTBC.
Comment #10
merlinofchaos commentedCan you reroll the patch so I don't have to manually go edit it out? =)
Comment #11
dkruglyak commentedYou got it
Comment #12
merlinofchaos commentedCommitted!