'#\[\*(?::\w+)?\](.+)(?=\[\*(?::\w+)?\]|\[/list)#Usi'   => '<li>\\1',
    '#\[list(?::\w+)?\]#si'                                 => '<ol class="bb-list" style="list-style-type:circle;">',
    '#\[list=c(?::\w+)?\]#si'                               => '<ol class="bb-list" style="list-style-type:circle;">',
    '#\[list=d(?::\w+)?\]#si'                               => '<ol class="bb-list" style="list-style-type:disc;">',
    '#\[list=s(?::\w+)?\]#si'                               => '<ol class="bb-list" style="list-style-type:square;">',

In these four lines you see ordered lists being used as unordered lists. The HTML tags should be changed to

    .
CommentFileSizeAuthor
#14 bbcode_5_patch_5-3.txt4.67 KBTakafumi
#12 bbcode_5_patch_5-2.txt4.67 KBTakafumi
#8 bbcode_5_patch_5.txt3.24 KBTakafumi

Comments

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented

The tags should be changed to <u>.

And again I forget HTML works on this forum and type the tags without encoding :P

naudefj’s picture

naudefj commented
Status: Active » Closed (works as designed)

What? The <u> tag is used for underlining text!

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented

Sorry, typo! I meant the <ul> tag of course :P

naudefj’s picture

naudefj commented
Status: Closed (works as designed) » Postponed

I'm not going to change this. However, I will reconsider if a good working patch is provided.

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented

Why not change it and why reconsider when somebody has spent loads of time on trying to figure out how to make patches? It's just changing four characters...

naudefj’s picture

naudefj commented

Reasons are:
1) You want it, not me.
2) There are more to it than just changing ol to ul. Try it and see what happens.

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented

1) I want it, yes. I think Drupal (and its modules) should output semantically correct code. I think that partly is the reason why Drupal is so popular. It's very easy to make it output valid and semantic markup code.
2) Now I see. To be honest I haven't really paid much attention to the regexps until you mentioned it's more work than simply replace a few characters. I'll take a closer look at it to see if I can make some new regexps.

Takafumi’s picture

Takafumi commented
StatusFileSize
new bbcode_5_patch_5.txt3.24 KB

I think that should fix this issue, because those are not valid XHTML. Moreover, I found a different bug in the process of creating a patch. That bug doesn't discriminate between [list=a/i] and [list=A/I].
A attached patch will fix these issues. Please review and apply it.

naudefj’s picture

naudefj commented
Version: 5.x-1.x-dev » master
Assigned: Unassigned » naudefj
Status: Postponed » Needs review

Thanks Takafumi.

Xano can you please test it for us?

naudefj’s picture

naudefj commented
Status: Needs review » Needs work

Nested tags don't render correctly after applying this patch. Example:

[list=s]
[*]x
[*][list=d]
   [*]x.1
   [*]x.2
   [/list]
[*]y
[*]z
[/list]

Please test all tags in file bbcode-test.txt.

Takafumi’s picture

Takafumi commented

oops, I will try it again :p

Takafumi’s picture

Takafumi commented
StatusFileSize
new bbcode_5_patch_5-2.txt4.67 KB

Coding was completed. Please test this patch thoroughly.
Various patterns which I tested are as follows.

[list][*]foo[*]bar[*]baz[/list]
[list][*]foo[list][*]bar[/list][*]baz[/list]

[hr]

[list]
  [*]list-1
  [list]
    [*]list-2
      [list]
        [*]list-3
          [list]
            [*]list-4
              [list]
                [*]list-5
              [/list]
          [/list]
      [/list]
  [/list]
[/list]

[hr]

[list]
  [*]list
    [list=c]
      [*]list=c
      [*]list=c
        [list=d]
        [*]list=d
          [list=s]
            [*]list=s
            [*]list=s
            [*]list=s
         [/list]
        [*]list=d
        [*]list=d
      [/list]
      [*]list=c
    [/list]
  [*]list
  [*]list
[/list]

[hr]

[list=1]
  [*]list=foo
    [list=s]
      [*]list=s
      [*]list=s
    [/list]
    [list=a]
      [*]list=a
        [list=I]
          [*]list=I
          [*]list=I
        [/list]
      [*]list=a
    [/list]
  [*]list=bar
    [list=A]
      [*]list=A
      [*]list=A
      [*]list=A
    [/list]
  [*]list=baz
    [list=d]
      [*]list=d
      [*]list=d
    [/list]
[/list]

[hr]

[list]
  [*]list
  [*]list
  [*]list
[/list]

[list=c]
  [*]list=c
  [*]list=c
  [*]list=c
[/list]

[list=d]
  [*]list=d
  [*]list=d
  [*]list=d
[/list]

[list=s]
  [*]list=s
  [*]list=s
  [*]list=s
[/list]

[list=1]
  [*]list=1
  [*]list=1
  [*]list=1
[/list]

[list=i]
  [*]list=i
  [*]list=i
  [*]list=i
[/list]

[list=I]
  [*]list=I
  [*]list=I
  [*]list=I
[/list]

[list=a]
  [*]list=a
  [*]list=a
  [*]list=a
[/list]

[list=A]
  [*]list=A
  [*]list=A
  [*]list=A
[/list]
Takafumi’s picture

Takafumi commented
Status: Needs work » Needs review

forgot status change :p

Takafumi’s picture

Takafumi commented
StatusFileSize
new bbcode_5_patch_5-3.txt4.67 KB

A patch was updated because small bug was found.

naudefj’s picture

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

Patch works & doesn't break anything else. Thank you very much!

PS: I hope you won't mind if I remove your "Code by Takafumi (Drupal Japan)" comments. I will give you credit when committing the patch and when the new version is released.

Takafumi’s picture

Takafumi commented

That's all right. You may remove my comments, if you want.

naudefj’s picture

naudefj commented
Status: Reviewed & tested by the community » Fixed

Patch applied - http://drupal.org/cvs?commit=68595

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)