<ol> tag never closed, thus indentation problem [#159833]

Comment	File	Size	Author
#13	directory.module.diff	57 bytes	mdlueck

Comment #1

mdlueck CreditAttribution: mdlueck commented 16 July 2007 at 20:50

Ahhh... when a document starts out with an < ol > tag, Directory pulls in that tag... and since it does not pull the entire ordered list, it does not pull the close of that ordered list from the document.

I will try to track down what snips a bit of text to build that string in directory and try to adjust it to not do that.

Log in or register to post comments

Comment #2

mdlueck CreditAttribution: mdlueck commented 16 July 2007 at 21:33

This bug seems to track back to the node area of Drupal core. Opened a bug report against node:

http://drupal.org/node/159852

Log in or register to post comments

Comment #3

beginner CreditAttribution: beginner commented 17 July 2007 at 06:15

Status:

Active

» Closed (won't fix)

This is a long standing problem with Drupal core.
The reason why it has been left for so long is that there is no easy / ideal solution to this bug.
Most people have learned to live with it, and make proper use of the  tag to control how the teaser will show.

There is little we can do at the contrib module level.
I'll follow up on the Drupal core issue.

Log in or register to post comments

Comment #4

beginner CreditAttribution: beginner commented 17 July 2007 at 13:26

Priority:	Normal	» Minor
Status:	Closed (won't fix)	» Active

The same problem exists in core : http://drupal.org/node/82572 , but it is less noticeable there because teasers are put in a pair of <div> tags. This does not solve the underlying problem but the browsers know how to handle broken XHTML, so the user doesn't notice a thing.
We can try the same here, but before we do, I need to think clearly about the direction the module will take. I won't spend time fixing bugs for a feature that may completely be removed.

see this issue http://drupal.org/node/125587 which may be related,
and #8 here: http://drupal.org/node/144743#comment-275314

I keep this issue open until a decision is made.

Log in or register to post comments

Comment #5

mdlueck CreditAttribution: mdlueck commented 17 July 2007 at 13:31

From http://drupal.org/node/159852

It's better not to mix contrib module issues and core issues.

OK, then I will add the HTML tag stripper to Directory as it is Directory that need not have HTML tags in directory entries. Accept the comment that there are valid reasons to have HTML tags in teasers in general.

Log in or register to post comments

Comment #6

beginner CreditAttribution: beginner commented 17 July 2007 at 13:46

I am not sure what you mean, but to be clear, for the reasons already mentioned I will not commit any tag-stripping patch.

Log in or register to post comments

Comment #7

beginner CreditAttribution: beginner commented 17 July 2007 at 13:47

But if you want to strip tags in the teasers in your site, you can do so. Within your theme, you can create your own display function. You can handle the data the way you want.
After all, I worked very hard in the last 2 months to make sure that every display function is themable.

Log in or register to post comments

Comment #8

beginner CreditAttribution: beginner commented 17 July 2007 at 14:04

#82572 was itself a duplicate of: http://drupal.org/node/54833. And there is some interesting code to look at there.

@mdlueck , the best thing you could do is follow up on that issue and help to make sure a fix is committed in core.

Log in or register to post comments

Comment #9

beginner CreditAttribution: beginner commented 17 July 2007 at 14:09

The fix will actually be part of Drupal 6. Yeah! :)

#58 submitted by Dries on May 21, 2007 - 00:43
Status: patch (ready to be committed) » fixed
I re-rolled this patch against CVS HEAD, and committed it. Thanks all.

Log in or register to post comments

Comment #10

mdlueck CreditAttribution: mdlueck commented 17 July 2007 at 14:09

Within your theme, you can create your own display function. You can handle the data the way you want.

???? You seem to be full of surprises... How would I strip tags from the teaser at theme level?

I can not think of a point to leaving tags like < ol > and < li > in the teaser that Directory is displaying... so "best to nuke the tags" and preserve the Directory output integrity, no?

Scratching my head... (20 years of working at the OS/LAN/App level... PHP and Drupal in general seems "another world"... and I can not say that I am enjoying the experience.)

Log in or register to post comments

Comment #11

beginner CreditAttribution: beginner commented 17 July 2007 at 14:23

All the theme_*() functions can be overridden at the theme level. Browse the Themer's handbooks for more information.
Those functions include php, so anything you can code is possible.

The following two directory views are not possible with the unmodified directory.module. But at the theme level, I override the functions so that I can use the php I need:
http://activistsolutions.org/directory/105
http://activistsolutions.org/directory/136

I am starting to repeat myself a bit too much, but I won't work anymore on this issue until I know what to do with that: http://drupal.org/node/144743#comment-275314

Log in or register to post comments

Comment #12

mdlueck CreditAttribution: mdlueck commented 17 July 2007 at 15:15

All the theme_*() functions can be overridden at the theme level.

Aaaahhhh, that is what is meant by "Themeable Functions". All right, I will attempt to stick theme_directory_listing() into the theme.

Log in or register to post comments

Comment #13

mdlueck CreditAttribution: mdlueck commented 20 July 2007 at 01:41

Status:

Active

» Reviewed & tested by the community

File	Size
directory.module.diff	57 bytes

One line of code solved the issue of HTML tags in the teaser. I think it looks mighty fine. Still do not know in the case of Directory why anyone would want HTML tags in the teaser text.

I have over ridden the function in our custom theme.

Sending the patch to help others, or maybe you would be so kind as to consider applying the patch provided.

Log in or register to post comments

Comment #14

beginner CreditAttribution: beginner commented 20 July 2007 at 02:08

Status:

Reviewed & tested by the community

» Needs work

http://drupal.org/patch/create
http://drupal.org/node/156119/revisions

Log in or register to post comments

Comment #15

beginner CreditAttribution: beginner commented 20 July 2007 at 02:09

I mean: http://drupal.org/node/156119

Log in or register to post comments

Comment #16

mdlueck CreditAttribution: mdlueck commented 20 July 2007 at 02:55

No prob, Like I said, 'tis one line of code. I am content with leaving it in the theme. It is up to you if you care to merge it into the module. You did not sound as if you would even want to.

I might add, it is not the code that needs work, the formalities of the patch is what needs the work, and I do not think I have the necessary tools on my machine to satisfy the requirements.