Bug 46553

Summary: Fix for applyFont() method of HSSFRichTextString
Product: POI Reporter: David Wang <davidcw>
Component: HSSFAssignee: POI Developers List <dev>
Status: RESOLVED FIXED    
Severity: normal CC: davidcw
Priority: P2    
Version: 3.5-dev   
Target Milestone: ---   
Hardware: PC   
OS: Windows Vista   
Attachments: Code that demonstrates the problem
Example problem code with the excel document it acts on.

Description David Wang 2009-01-17 06:57:10 UTC
Created attachment 23137 [details]
Code that demonstrates the problem

I believe I found a bug in the applyFont() method of class HSSFRichTextString. I am also posting the corresponding fix. I am also including posting think demo code is necessary for this because 



Incidently, in searching for a related bug to which I could apply this fix, I found a similar bug: Bug 40520, "HSSFFont.applyFont() formats wrong parts of HSSFRichTextString". 

It looks like this bug has already been resolved in release 3.5-beta4. (I didn't test if it was resolved in 3.2 FINAL). I'm not fully familiar with the posting rules, so I didn't mark it as such.
Comment 1 David Wang 2009-01-17 07:11:28 UTC
Apologies, I accidently submitted this bug by clicking the Commit button prematurely. Here is the completed report...

I believe I found a bug in the applyFont() method of class HSSFRichTextString.
The fix for the applyFont() method is included in plain-text, below. The fix is simple: In the applyFont() method, in the section of code that "reapplies" the current font, it samples the font at the "startIndex". However, it would seem to make more sense if it samples the font at the "endIndex", and reapplies that instead.

I have also attached the code that demonstrates this bug, this time with the associated excel file.

This 'bug' exists in both release 3.2 FINAL and 3.5-beta4.

PROPOSED FIX:
============================
    public void applyFont(int startIndex, int endIndex, short fontIndex)
    {
        if (startIndex > endIndex)
            throw new IllegalArgumentException("Start index must be less than end index.");
        if (startIndex < 0 || endIndex > length())
            throw new IllegalArgumentException("Start and end index not in range.");
        if (startIndex == endIndex)
            return;

        //Need to check what the font is currently, so we can reapply it after
        //the range is completed
        short currentFont = NO_FONT;
        if (endIndex != length()) {
          // FIX: USES "endIndex" instead of the original "startIndex".
          currentFont = this.getFontAtIndex(endIndex); 
        }

        //Need to clear the current formatting between the startIndex and endIndex
        string = cloneStringIfRequired();
        Iterator formatting = string.formatIterator();
        if (formatting != null) {
          while (formatting.hasNext()) {
            UnicodeString.FormatRun r = (UnicodeString.FormatRun)formatting.next();
            if ((r.getCharacterPos() >= startIndex) && (r.getCharacterPos() < endIndex))
              formatting.remove();
          }
        }


        string.addFormatRun(new UnicodeString.FormatRun((short)startIndex, fontIndex));
        if (endIndex != length())
          string.addFormatRun(new UnicodeString.FormatRun((short)endIndex, currentFont));
          
        addToSSTIfRequired();
    }


============================

Incidently, in searching for a related bug to which I could apply this fix, I
found a similar bug: Bug 40520, "HSSFFont.applyFont() formats wrong parts of
HSSFRichTextString". 

It looks like this bug has already been resolved in release 3.5-beta4. (I
didn't test if it was resolved in 3.2 FINAL). I'm not fully familiar with the
posting rules, so I didn't mark it as such. 
Comment 2 David Wang 2009-01-17 07:12:57 UTC
Created attachment 23138 [details]
Example problem code with the excel document it acts on.
Comment 3 Yegor Kozlov 2009-01-29 08:06:16 UTC
the fix applied in r738908.

Thanks,
Yegor