I do not know why on Earth people think that the <br> tag when written in proper XML needs a space as in <br /> since it is not only optional, but if you have to write 1,024 of them all of a sudden you lose 1Kb of data for nothing.

In any event, that's my rant! 8-)

That being said, since <br/> is perfectly legal and I use it all the time, you need to fix your code in the node_teaser() function and check for that case too.

And as you are at it, you may want to back-port to version 6.x

Thank you.
Alexis Wilke

CommentFileSizeAuthor
#5 354319.patch2.92 KBdawehner
#3 354319.patch810 byteslambic
node-7.x-br-n-space.patch490 bytesAlexisWilke

Comments

Status: Needs review » Needs work

The last submitted patch failed testing.

AlexisWilke’s picture

AlexisWilke commented

Okay... Could someone tell me what test is failing?

When I click on the "Detailed results" it shows me the results, but no information about the test.

Plus, if the test is wrong (which it has to be, really!) how will this be ever checked in???

lambic’s picture

lambic commented
Status: Needs work » Needs review
StatusFileSize
new 354319.patch810 bytes

looks like this was moved to the field module, rerolled against HEAD and as a root level patch

AlexisWilke’s picture

AlexisWilke commented

Thank you lambic! I thought no one had seen this bug... 8-)

dawehner’s picture

dawehner
Primary language German
commented
StatusFileSize
new 354319.patch2.92 KB

So if we introduce it, shouldn't there be a test for it too?

roychri’s picture

roychri commented
Status: Needs review » Reviewed & tested by the community

I tried so many scenarios to try to break it and I did not find anything wrong.

RTBC

dries’s picture

dries commented

From http://en.wikipedia.org/wiki/User:Davidgothberg/The_br_tag:

Writing the XHTML code <br/> without a blank is against the recommendations of the World Wide Web Consortium, instead it should be written as <br /> since then HTML parsers can understand it too. HTML parsers will simply regard <br /> as a "br" with an unknown parameter "/", while they will regard "br/" as an unknown tag name. So we should definitely not teach people to write <br/>, but possibly <br />.

So not sure this patch is a good idea...

roychri’s picture

roychri commented

Hmmm. Interesting...

From the consortium:
http://www.w3.org/TR/xhtml1/guidelines.html#C_2

Include a space before the trailing / and > of empty elements, e.g. <br />, <hr /> and <img src="karen.jpg" alt="Karen" />. Also, use the minimized tag syntax for empty elements, e.g. <br />, as the alternative syntax <br></br> allowed by XML gives uncertain results in many existing user agents.

AlexisWilke’s picture

AlexisWilke commented

Dries,

I have never seen any HTML browser fail on a <br> or <br/>. Maybe some other HTML tools, but that I don't care, we're talking about Drupal here. Also the link you offer does not work.

roychri,

Interesting indeed. I had not seen that one. Now, it is okay, but for those who look at the XML definition (like me) they are likely to miss that one line. Also, that Appendix C is marked as follow: "This appendix is informative." That means for a tool like Drupal, you may want to be a bit more advanced...

All,

So... The Empty tag is defined here:

http://www.w3.org/TR/2006/REC-xml11-20060816/#NT-EmptyElemTag

EmptyElemTag	   ::=   	'<' Name (S  Attribute)* S? '/>'

As you can see the space is optional (as implied by the question mark.)

Now what this patch allows is silly users like me to write <br/> without a space, it does not promote the fact that you should do that. The HTML actually looks very broken if you skip those tags. And for people who already have content and want to start with Drupal doing some copy & paste of their existing pages, it would be a pain to have to fix all the BR tags just because of a space.

Finally, if you are worried about the browsers getting <br/> tags and failing, we could always have a filter to fix it (and all the empty tags if that matter.) That way, we allow users flexibility, but browsers conformity. (best of both worlds!)

dries’s picture

dries commented
Status: Reviewed & tested by the community » Needs work

I think more people write <br /> than <br/>. The proposed fix works for some but breaks the behavior of users that do the right thing. We'll need to go back to the drawing board on this -- and it will have to be a Drupal 8 thing, I'm afraid.

drupal_was_my_past’s picture

drupal_was_my_past commented
Status: Needs work » Closed (won't fix)

This issue is stale and had two strong arguments against it. Marking won't fix.