The recent and controversial break tag change have not included blogapi which still has the HTML comment style break tag.

We many resolutions and before raining this issue with patches, let's first decide which avenue we take

  1. We use a Drupal specific pseudo tag, hardwired. There seems to be little support for this -- even in the original issue I saw only two.
  2. We use a HTML comment tag, hardwired. Given one of the aforementioned two persons, the chances for .this is nil. But note that blogapi still has has tag hence this bug report.
  3. We use a variable without an interface which defaults to the Drupal specific pseudo tag.
  4. We use a variable without an interface which defaults to the HTML comment tag and we remove the update altogether.
  5. We use a variable without an interface which defaults to the HTML comment tag and we provide an update which reverts system_update_1018.

Code is written in http://drupal.org/node/87145 for the last three choices, so it's just a matter of decision.

This itme, let's wait for Dries decision and when he has made up his mind then we submit the patch from that issue.

CommentFileSizeAuthor
#20 rollback_0.patch3.38 KBchx
#9 break_4.patch5.66 KBSteven
#4 break_3.patch3.16 KBSteven
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

codepoet’s picture

I vote #4, FWIW.

andrewfn’s picture

#4 gets my vote

Steven’s picture

Status: Active » Needs review

Usability over backwards compatibility: the short tag should stay.

I also find this whole thing rather ironic. The thread where XHTML validity was the prime argument, got closed because someone screwed up the HTML.

And now, you want to change it because it was added by one core committer against the will of several people, yet propose even more complicated mechanisms to replace it. And then, you expect another core committer to make the decision.

My vote is to just fix the original bug: blogapi.module. Patch attached. I also removed a freak occurance of the break tag in the help text, which has no use as far as I can tell.

The break tag has been debated over and over, and has wasted too much time. We have more productive things to do.

Steven’s picture

FileSize
3.16 KB

*stupid losing attachment on preview bug*

m3avrck’s picture

I agree with Steven, this patch looks good to me.

RobRoy’s picture

Status: Needs review » Needs work

We write ".''. $node->body;" not ".''.$node->body;" :P I know it's in the original, but might as well fix it while we're in there. I would re-roll, but I'd be betraying my beliefs.

Even though there are some bad points in that thread, there are still some good ones. You/we should focus on refuting or confirming the VALID points for rolling this back if we actually want to get somewhere, not just bashing the poor arguments as there are quite a few.

RobRoy’s picture

There is just too much irony in this world, should have encoded that break tag. If only it was more usable... :D

eaton’s picture

Since it was either missed or ignored, I'll simply post dman's comment from http://drupal.org/node/105278

Well, the data in my database always was. You can say it's not by design, but it was a good thing when it was. This one change now deliberately makes it invalid for what I see as a trivial win. Fixing a problem that wasn't broken (HTML comment syntax confuses some people) by destroying the ability to work with modern XML tools and such.

Much of my work - contributions to the HTML tidy module - The import html module - XSL filter module - etc all enforce good, valid, syntax on the text they work with. Usually this means the unfiltered database stuff. My input validator for htmltidy for example prevents bad HTML from getting into the DB in the first case. This new made-up pseudo-tag is bad HTML.
I've done lots of XML, lots of XSL, Lots of DTDs, Namespaces and related stuff. I now appreciate why these annoying, strict validation rules are required, and why they are a good thing.

I've spent a decade migrating and translating old handmade or custom-made sites from one system to another to another. Web-wide, Early hacks often made up their own tags then did on-the-fly substitutions. This may have worked for them, but produced crap migration, editing, validation and maintainability issues.
Eventually some of the editors stopped doing that and Frontpage and Dreamweaver etc started hiding their custom extensions in comments. Yeah it's messy, but it didn't break everyone elses way of doing things and the source files were mostly portable again.

With all of the validation tools and standards available to us nowadays, and lessons learnt from past problems, I think it's a great leap backwards to deliberately break what was (at least potentially) otherwise perfectly valid source. Modern practices may allow you to legally extend your custom additions via namespaces but I don't think that'll please either side of this discussion.

[snip]

Sorry, I've done too much content conversion between different CMSs on different platforms to get tied down to the my-database-is-the-only-answer lock-in. Thinking your pages will only ever exist inside Drupal with its output filters always applied is short-sighted.

Imagine a client-side HTML editor that could be tied into edit drupal pages via RPC or WEBDAV, much like some current blog tools or FTP-capable text editors already do. This one invalidating change deliberately breaks compatability with such a system.

I think either keeping the comment tag version, or(and) supporting the visual markdown [break] version to help the rich-text editors is fine.

OK. Maybe this post should have been in the issue thread, but my rant is about the continuing validity of my data, as per this thread, not about the UI discussions that went on there.

[snip]

I want the node data to be reasonably self-contained. Asking for something that is currently 100% valid HTML to continue to be valid HTML is not a big ask. Deliberately breaking it for no big win like this change does seems wrong.

There are infinite validating alternatives.
The comment version was fine.
<br class='pagebreak' /> would work for me.
wiki-like-syntax --- would be fine. markdown [break] is not bad.

WHY does the pseudo-HTML choice have to be the one thing that would instantly invalidate HTML, XHTML, wysiwyg editors, third party tools, future upgrades and everything?

I think it's pretty well established that there's no way the break patch will be rolled back or fixed. But it's worth explaining to dman why his work at data validation is not worth our consideration.

Steven’s picture

Status: Needs work » Needs review
FileSize
5.66 KB

Any direct use of the content in the node table requires post processing. Either you use the drupal filter system, or you roll your own. In the latter case, a simple str_replace is all that is needed. it is not the end of the world.

Here's a patch which should fix all bad code style regarding concatenation, given that you're playing the pedantic card, and still addresses the break tag bug.

chx’s picture

Status: Needs review » Active

Before others post patches for all five -- no need to reopen the debate. Now we have code for 1.) in this issue. Good. I am moving this to active because let's wait for Dries decision and when he has made up his mind then submit the patch. I do not want another 100 followup long thread with totally different patches being thrown in.

ontwerpwerk’s picture

option 4 or 5 are my choices - because they:

  • don't break backwards compatility
  • don't make existing users relearn their habits
  • are not introducing ambiguity about the <br> and <break> which would both be pronounced "break tag" on the phone to a user
  • are valid in html if the drupal filters are bypassed (the drupal database is not always an island, sometimes there are bridges, or sometimes the content is exported)
  • are flexible for people who need something else (a module providing an interface to change it and some functions that update the content is easy to make)

ultimately I would say excerpt.module or something similar should be in core to make this whole discussion disappear

dman’s picture

Thanks to Eaton for going in to bat and x-posting my rant.
I'm simply casting my vote and if I'm out-voted (or, um, over-ruled due to status) I'll live with it.

Yes, I can write a regexp just to deal with this one special case. And the next one, and the next one that the Drupal majority sees fit to inflict upon us. But I'll regard it as a failing of the Drupal data storage mechanism that it requires me to do so each time, and simultaneously prevents me from using the right tools to do so. Even the classic HTML 'tidy' tool will now totally barf on this unrecognised tag, and that can deal with almost any other bad data and markup the net (or Microsoft) could ever throw at it.

Either you use the drupal filter system, or you roll your own

... you gotta try migrating some legacy content through several consecutive flavour-of-the-year formats and platforms some time. Heaps of fun.
Everyone thinks their own justification for ignoring standards is valid, and the folk who learnt the hard way are pedantic. *sigh*
Discard all the work done 'till now on getting XHTML to finally work right because you want to add one unneccessary custom flourish of your own and invent the incompatable DrupalML? please...

Data format lock-in (proprietary encoding of my data) like you dictate here when a compatable, valid, better, open, formalized alternative (XHTML) is ignored is enough to make most folk drop a software package altogether.
Please lets not be different just to be awkward.

.dan.

dman’s picture

... however, to get out of rant mode and try and be constructive again...

I vote 4 or 5, But why not provide an interface to this variable so that admins who wanted to could just replace this marker with anything of their own choosing? In their own language even.
If they wanted to deal with the consequences (or more specifically wanted the poor bugger who came 2 years after them to deal with the consequences) and added invalid HTML, so be it.

It may be a cop-out according to some UI guidelines ;-) but if you can't get a solution that pleases everyone, just make it configurable under advanced options :-B

.dan.

webchick’s picture

The reason we can't really provide an interface to such an option is because every single node in the database would need to be updated each time this was changed, which is extremely expensive. So it should really only be something you only do once, but if it's available for users to click on they probably will. ;P

http://drupal.org/node/107061 is an attempt at a compromise which both retains the original break comment and improves usability for end-users. See what you think.

dman’s picture

So it should really only be something you only do once, but if it's available for users to click on they probably will. ;P

True 'nuff.
... so don't do that then ;-)

Bad things happen if you change your files directory on-the-fly. So don't do that.
Random things may happen if you change the site theme without testing. Admins should not mess with it unless they really want to.
Changing the thumbnail size of images half-way through a live site will produce inconsistant results. Don't do that!

This config is not that much different.
It's for first-set-up admins, not users.
It's a really advanced admin decision, and should not be messed with... more than once.

Granted, It may be expensive to do a global search+replace on all nodes once (if that added option were even provided), but it's an admin function, not something that happens regularly.

You have a point, but not a great argument.

chx’s picture

Status: Active » Closed (duplicate)
dman’s picture

I apologise for not reading to the bottom of the most recent http://drupal.org/node/87145 issue before posting above.
I'm happy with the default behaviour + optional super-admin (settings.php) override option (#4?) as seemed to be the outcome of that thread. Lets fry some bigger fish now eh?

dman’s picture

Apologies for not totally reading each of the 5 strands this topic has now taken. The 'non-configuration' option of just setting it in the settings.php (of course, doh) is fine by me. I was reading the 'no interface' limitation a bit harshly. Sure, let it be set there ... at your own risk :-)

ericG’s picture

primary concerns should about usability as well as using standards.

usability for long term drupal users means not changing something so critical to their daily use of their site.

usability for site admins means not melting their database servers on an upgrade modifying node content needlessly

usability for developers means adherence to current web standards

all that means my vote is for #2, but am willing to see #4 as a valid compromise

If you want to do strange things to how the database stores menu info, url aliases, or anything that is truly drupal specific, then fine I have no problem with that, but changing the node content of people's sites to appease one stouborn developer? that is unacceptable. Do what you want with the rest of the db, leave my user-entered content alone!

chx’s picture

Title: Break tag is inconsistent » Roll back break tag
Status: Closed (duplicate) » Needs review
FileSize
3.38 KB

http://lists.drupal.org/archives/development/2007-01/msg00159.html states that Steven's JS won't go into 5.x and asks for choosing between options one or two and a clean up.

But there is a new argument on the field: blogapi.

Our support for blog APIs mandate a roll back. If I have a client which does a massive post to several sytems at once and the client supports Drupal then it could include an HTML comment to create a teaser for Drupal but it can not really insert a pseudotag as that would immediately break other system. I do not believe that forcing such clients to do extensive work to find whether the other end is a Drupal and insert the tag only if it finds a Drupal is acceptable. It's known to be rather hard to fingerprint a Drupal system if the administrator chooses to conceal this fact.

So the only acceptable solution is to roll back.

Caleb G2’s picture

There is widespread agreement that either option, a) changing the break-command OR b) rolling it back is a temporary situation. The teaser has already been targeted to major change in Drupal 6 - this much is known.

So common sense is to use which ever temporary solution is less trouble to deal with while we work on fixing it the right way. Obviously not changing the way the break command has been worked for years is easier than changing it to something else for one release.

Lets please either: a) take the time to do it right NOW and be happy about it, or b) if that's not an option wait for calmer times (post 5.0 release) and do it right later.

Paul Natsuo Kishimoto’s picture

Standards incompliance also breaks future and present compatibility. Correct

Usability over backwards compatibility: the short tag should stay.

and the nonsensical result is:

Usability over compatibility: the short tag should stay.

Compatibility and usability are inseparable; and you can't advance one by removing the other.

Voting #4, because a non-interface variable enables site admins to supplant <!--break--> if they feel the extra five characters will clog their databases and 'lopsidedness' will cause users to claw out their eyeballs. Those with 'more productive things to do' can leave <!--break--> and immediately hook their valid XHTML content up to non-Drupal APIs.

ericG’s picture

re #20

http://lists.drupal.org/archives/development/2007-01/msg00159.html states that Steven's JS won't go into 5.x and asks for choosing between options one or two and a clean up.

When I read that post on the mailing list I read it as just another developer giving their opinion. I did not realize that it was the announcement of a decision and narrowing of the options. Thanks for being more explicit on what was between the lines of that post.

In that case, I'll just vote for a rollback to hardcoded as a comment tag (#2, although #4 is still a fine compromise), and go back to working on patches I've promised to some module maintainers.

webchick’s picture

Status: Needs review » Fixed

This was rolled back in CVS.

webchick’s picture

Status: Fixed » Closed (fixed)

And closing this, to put this nasty stuff behind us.

Please see http://drupal.org/node/107061 if you're interested in making teasers kick-ass for 6.x!