Skip to content
This repository has been archived by the owner on Nov 24, 2020. It is now read-only.

Border style unintentionally shared between XSSFStyle instances #157

Open
vpstuart opened this issue Dec 20, 2019 · 2 comments
Open

Border style unintentionally shared between XSSFStyle instances #157

vpstuart opened this issue Dec 20, 2019 · 2 comments

Comments

@vpstuart
Copy link

vpstuart commented Dec 20, 2019

I have found in an application that the border formatting in XSSFCellStyle is a bit broken. It results in accidentally sharing a CT_Border instance.

From https://github.com/dotnetcore/NPOI/blob/master/src/NPOI.OOXML/XSSF/UserModel/XSSFCellStyle.cs#L227

        public BorderStyle BorderBottom
        {
            get
            {
                if (!_cellXf.applyBorder) return BorderStyle.None;

                int idx = (int)_cellXf.borderId;
                CT_Border ct = _stylesSource.GetBorderAt(idx).GetCTBorder();
                if (!ct.IsSetBottom())
                {
                    return BorderStyle.None;
                }
                else
                {
                    return (BorderStyle)ct.bottom.style;
                }
            }
            set
            {
                CT_Border ct = GetCTBorder();
                CT_BorderPr pr = ct.IsSetBottom() ? ct.bottom : ct.AddNewBottom();
                if (value == BorderStyle.None) 
                    ct.UnsetBottom();
                else 
                    pr.style = (ST_BorderStyle)value;

                int idx = _stylesSource.PutBorder(new XSSFCellBorder(ct, _theme));

                _cellXf.borderId = (uint)idx;
                _cellXf.applyBorder = (true);
            }
        }

Each of the border setters has the same but, I have chosen BorderBottom as an example.

The first time you assign a value to a border property. E.g. cell.BorderBottom = BorderStyle.Thin; it will create a CT_Border object, assign the border to it and then call _stylesSource.PutBorder(new XSSFCellBorder(ct, _theme)) which will store the border object and return an ID.

If it finds a matching CT_Border it will return that instances id, causing it to be shared.

This seems to be from here:
https://github.com/dotnetcore/NPOI/blob/master/src/NPOI.OOXML/XSSF/Model/StylesTable.cs#L285

        public int PutBorder(XSSFCellBorder border)
        {
            int idx = borders.IndexOf(border);
            if (idx != -1)
            {
                return idx;
            }
            borders.Add(border);
            border.SetThemesTable(theme);
            return borders.Count - 1;
        }

Using borders.IndexOf(border) will search by the current value of the border. The XSSFCellBorder class implements equality based on the XML value of the CT_Border.

I think the easiest and possibly best solution would be to delete the 5 lines:

            int idx = borders.IndexOf(border);
            if (idx != -1)
            {
                return idx;
            }

I'm not a regular contributer here, but if it would help I can try and put together a PR.

Warm Regards,
Stuart

@vpstuart
Copy link
Author

On second look that will result in the pool of borders growing each time. Perhaps instead CT_Border ct = GetCTBorder(); needs to always make a clone.

@gbs56
Copy link

gbs56 commented Dec 26, 2019

I didn't know the cause (what you describe) but this was happening to me. Define a border in one style affects any other style that has border definition.
Good finding.
Thanks,
Gustavo

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants