veger / ruby-bbcode

Convert BBCode to HTML and check whether the BBCode is valid
http://rubygems.org/gems/ruby-bbcode
MIT License
28 stars 29 forks source link

Uppercased tag not parsed by bbcode_to_html method. #27

Closed gitoido closed 9 years ago

gitoido commented 9 years ago

So, here it is, as described in issue title. /test/ruby_bbcode_bbcode_test.rb This test passes.

def test_uppercased_tags
  assert_equal '[B]simple[/B]', '[B]simple[/B]'.bbcode_show_errors
  assert_equal "[B]line 1\nline 2[/B]", "[B]line 1\nline 2[/B]".bbcode_show_errors
end

But this is not. /test/ruby_bbcode_html_test.rb

def test_uppercased_tags
  assert_equal '<strong>simple</strong>', '[B]simple[/B]'.bbcode_to_html
  assert_equal "<strong>line 1<br />\nline 2</strong>", "[B]line 1\nline 2[/B]".bbcode_to_html
end

UPD:

My guess that is method RubyBBCode#determine_applicable_tags has to do something with that. Clone final hash of bbcodes with uppercased keys maybe?

TheNotary commented 9 years ago

Ah, you found two bugs sort of. When a tag is provided in uppercase, it looks for the tag definition in @@tags, but those are all provided in lowercase. When it can't find any tags in lower case, it thinks the tag isn't defined, and therefore treats it as just normal text.

Solution is that the tests that use #bbcode_show_errors should be deprecated in favor of they style you wrote (when it makes sense).

To debug the situation, I took a look at TagSifter (where bbcode data is sifted through and sorted out into an abstract syntax tree type thingy) and noticed this line and figured it was probably where things were going wrong (notice the last check on that line)

https://github.com/veger/ruby-bbcode/blob/master/lib/ruby-bbcode/tag_sifter.rb#L33

After following up I came to:

https://github.com/veger/ruby-bbcode/blob/master/lib/ruby-bbcode/tag_info.rb#L105

I'm not setup to do development on this laptop, but try changing that line to downcase the tag before looking it up in the dicitonary, like this

@definition = dictionary[ti[:tag].downcase]
gitoido commented 9 years ago

@TheNotary, Yep, that did the trick. Tests all green now https://github.com/warezgibzzz/ruby-bbcode/blob/master/test/ruby_bbcode_html_test.rb#L10

veger commented 9 years ago

I never used uppercase tags, so did not found this issue... :) I think it will be good improvement to also support uppercase tags.

If you provide a PR with the fix and some additional tests with uppercase tags, I'll review it and merge when it is ok.