Merge pull request #70 from chi-deutschland/fix-worksheet-title-length-rule

Fix worksheet title length enforcement

3 files changed, 20 insertions, 19 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2a2aba0a..2dd0ebbd 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,7 @@ CHANGELOG
   - [PR #62](https://github.com/caxlsx/caxlsx/pull/62) - Fix edge cases in format detection for objects whose string representation made them look like numbers but the object didn’t respond to `#to_i` or `#to_f`.
   - [PR #56](https://github.com/caxlsx/caxlsx/pull/56) - Add `zip_command` option to `#serialize` for faster serialization of large Excel files by using a zip binary
   - [PR #54](https://github.com/caxlsx/caxlsx/pull/54) - Fix type detection for floats with out-of-rage exponents
+  - [I #67](https://github.com/caxlsx/caxlsx/issues/67) - Fix regression in worksheet name length enforcement: Some unicode characters were counted incorrectly, so that names that previously worked fine now stopped working. (This was introduced in 3.0.2)
   
 - **July.16.20**: 3.0.2
   - [I #51](https://github.com/caxlsx/caxlsx/issues/51) - Images do not import on Windows. IO read set explicitly to binary mode.
diff --git a/lib/axlsx/workbook/worksheet/worksheet.rb b/lib/axlsx/workbook/worksheet/worksheet.rb
index aae91947..9f3966f8 100644
--- a/lib/axlsx/workbook/worksheet/worksheet.rb
+++ b/lib/axlsx/workbook/worksheet/worksheet.rb
@@ -663,7 +663,9 @@ module Axlsx
 
     def validate_sheet_name(name)
       DataTypeValidator.validate :worksheet_name, String, name
-      raise ArgumentError, (ERR_SHEET_NAME_TOO_LONG % name) if name.bytesize > 31
+      # ignore first character (BOM) after encoding to utf16 because Excel does so, too.
+      character_length = name.encode("utf-16")[1..-1].encode("utf-16").bytesize / 2
+      raise ArgumentError, (ERR_SHEET_NAME_TOO_LONG % name) if character_length > 31
       raise ArgumentError, (ERR_SHEET_NAME_CHARACTER_FORBIDDEN % name) if '[]*/\?:'.chars.any? { |char| name.include? char }
       name = Axlsx::coder.encode(name)
       sheet_names = @workbook.worksheets.reject { |s| s == self }.map { |s| s.name }
diff --git a/test/workbook/worksheet/tc_worksheet.rb b/test/workbook/worksheet/tc_worksheet.rb
index 781f5250..734096b5 100644
--- a/test/workbook/worksheet/tc_worksheet.rb
+++ b/test/workbook/worksheet/tc_worksheet.rb
@@ -29,15 +29,28 @@ class TestWorksheet < Test::Unit::TestCase
     assert_raises(ArgumentError) { @ws.name = 'foo?bar' }
   end
 
+  def test_name_unique
+    assert_raise(ArgumentError, "worksheet name must be unique") { n = @ws.name; @ws.workbook.add_worksheet(:name=> n) }
+  end
+
+  def test_name_unique_only_checks_other_worksheet_names
+    assert_nothing_raised { @ws.name = @ws.name }
+    assert_nothing_raised { Axlsx::Package.new.workbook.add_worksheet :name => 'Sheet1' }
+  end
+
   def test_exception_if_name_too_long
     assert_nothing_raised { @ws.name = 'x' * 31 }
     assert_raises(ArgumentError) { @ws.name = 'x' * 32 }
   end
 
   def test_exception_if_name_too_long_because_of_multibyte_characters
-    three_byte_character = "✔"
-    assert_nothing_raised { @ws.name = 'x' * 28 + three_byte_character}
-    assert_raises(ArgumentError) { @ws.name = 'x' * 29 + three_byte_character }
+    four_characters_for_excel = "\u{1F1EB 1F1F7}" # french flag emoji
+    assert_raises(ArgumentError, "name too long!") do
+      @ws.name = four_characters_for_excel + "x" * 28
+    end
+    assert_nothing_raised { @ws.name = "#{four_characters_for_excel}123456789012345678901234567" }
+    assert_nothing_raised { @ws.name = "123456789012345678901234567890…" }
+    assert_nothing_raised { @ws.name = "123456789012345678901234567890✔" }
   end
 
   def test_page_margins
@@ -460,21 +473,6 @@ class TestWorksheet < Test::Unit::TestCase
     assert_equal(@ws.relationships.size, 4, "adding a pivot table adds 1 relationship")
   end
 
-
-  def test_name_unique
-    assert_raise(ArgumentError, "worksheet name must be unique") { n = @ws.name; @ws.workbook.add_worksheet(:name=> n) }
-  end
-
-  def test_name_unique_only_checks_other_worksheet_names
-    assert_nothing_raised { @ws.name = @ws.name }
-    assert_nothing_raised { Axlsx::Package.new.workbook.add_worksheet :name => 'Sheet1' }
-  end
-
-  def test_name_size
-    assert_raise(ArgumentError, "name too long!") { @ws.name = Array.new(32, "A").join() }
-    assert_nothing_raised { @ws.name = Array.new(31, "A").join() }
-  end
-
   def test_set_fixed_width_column
     @ws.add_row ["mule", "donkey", "horse"], :widths => [20, :ignore, nil]
     assert(@ws.column_info.size == 3, "a data item for each column")