Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GetComment() returns incorrect mapping between worksheets and comments #345

Closed
tobiassoppa opened this issue Feb 21, 2019 · 6 comments
Closed

Comments

@tobiassoppa
Copy link

Description
func (f *File) GetComments() (comments map[string][]Comment) returns wrong mapping between worksheets and comments.

This problem only occurs if a workbook contains multiple worksheets and at least one of them (except the last one) doesn't contain comments.

Example:
Assuming a workbook has 3 worksheets, there will be 3 files in \xl\worksheets.
sheet1.xml (sheet without comments), sheet2.xml and sheet3.xml.
Since the first sheet doesn't contain any comments, there will only be two comment files in \xl\.
comments1.xml, comments2.xml.

Currently the relation between sheet<number>.xml and comments<number>.xml seems to be index based, according to Comment.go.

Code snippet from link above:
commentID := f.GetSheetIndex(n)
commentsXML := "xl/comments" + strconv.Itoa(commentID) + ".xml"

Solution:
Matching sheet<number>.xml and comments<number>.xml based on a common index returns wrong matchings, if any worksheet (except the last one) has no comments.
It seems that the proper relationship between worksheets and comments can be found in \xl\worksheets\_rels.

This folder contains XML files for each worksheet that has any relationships. A plain worksheet will have no file in \xl\worksheets\_rels.

According to the example I mentioned above,
sheet1.xml.rels contains the lines

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<Relationships xmlns="http://schemas.openxmlformats.org/package/2006/relationships">
                <Relationship Id="rId1" Type="http://schemas.openxmlformats.org/officeDocument/2006/relationships/table" Target="../tables/table1.xml"/>
</Relationships>

Note that there is no reference to \xl\comments1.xml.

sheet2.xml.rels contains the lines

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<Relationships xmlns="http://schemas.openxmlformats.org/package/2006/relationships">
                <Relationship Id="rId2" Type="http://schemas.openxmlformats.org/officeDocument/2006/relationships/comments" Target="../comments1.xml"/>
                <Relationship Id="rId1" Type="http://schemas.openxmlformats.org/officeDocument/2006/relationships/vmlDrawing" Target="../drawings/vmlDrawing1.vml"/>
</Relationships>

Note that there is a reference to \xl\comments1.xml in line

<Relationship Id="rId2" Type="http://schemas.openxmlformats.org/officeDocument/2006/relationships/comments" Target="../comments1.xml"/>

To avoid the issue, I would propose to match worksheets and comments based on their relationship in
\xl\worksheets\_rels. Please note that a plain worksheet will not have a corresponding file in \xl\worksheets\_rels. Thus it may happen that you will only see sheet2.xml.rels and sheet3.xml.rels, assuming that worksheet one is plain, but worksheet two and three contain comments.

Steps to reproduce the issue:

  1. Create a new Excel file
  2. In case you don't have three worksheets by default, create two additional sheets.
  3. Set a comment in any cell on worksheet 2 and worksheet 3. Don't set any comment on worksheet 1!
  4. Call GetComments()

Describe the results you received:
Output with some additional formatting:

Sheet1 [{xxx, xxx 0 A1 This} {xxx, xxx 0 B1 Is} {xxx, xxx 0 C1 A test comment}]
Sheet2 [{xxx, xxx 0 A1 Another} {xxx, xxx 0 B1 Test} {xxx, xxx 0 C1 Comment}]
Sheet3 []

Describe the results you expected:
Since the first sheet had no comments, but only sheet two and three, I would expect:

Sheet1 []
Sheet2 [{xxx, xxx 0 A1 This} {xxx, xxx 0 B1 Is} {xxx, xxx 0 C1 A test comment}]
Sheet3 [{xxx, xxx 0 A1 Another} {xxx, xxx 0 B1 Test} {xxx, xxx 0 C1 Comment}]

Output of go version:

go version go1.11.5 darwin/amd64

Excelize version or commit ID:

v1.4.1

Environment details (OS, Microsoft Excel™ version, physical, etc.):
OSX Mojave 10.14.3
Microsoft Excel 2010

@xuri xuri added the confirmed This issue can be reproduced label Feb 23, 2019
@xuri xuri closed this as completed in c223815 Feb 23, 2019
@xuri
Copy link
Member

xuri commented Feb 23, 2019

Hi @tobiassoppa, thanks for your issue. I have fixed it.

@xuri xuri removed the confirmed This issue can be reproduced label Feb 23, 2019
@xyz347
Copy link

xyz347 commented Mar 30, 2020

This appeared again in 2.1.0

@xuri
Copy link
Member

xuri commented Mar 30, 2020

Hi @xyz347, thanks for your feedback. I have tested and it works well. Could you provide file and code attachments without sensitive info?

@xyz347
Copy link

xyz347 commented Mar 30, 2020

a.xlsx

package main

import (
        "fmt"
        "github.com/360EntSecGroup-Skylar/excelize/v2"
)

func main() {
        a, err := excelize.OpenFile("a.xlsx")
        fmt.Printf("err=%v\ncomments=%+v\n", err, a.GetComments())
}

output is (miss sheet a1)
comments=map[a2:[{Author:hxy AuthorID:0 Ref:A1 Text:nice}]]

if use v2.0.0, output is ok:
comments=map[a1:[{Author:hxy AuthorID:0 Ref:A1 Text:hi}] a2:[{Author:hxy AuthorID:0 Ref:A1 Text:nice}]]


How to reproduce the bug:
add sheet a1/a2 and add comment
add sheet nc without comment
move sheet nc after a1

sometimes map miss some sheet
sometimes map's key and value is wrong

@xuri
Copy link
Member

xuri commented Mar 30, 2020

I have fixed it. Please try to upgrade the library with the master branch code.

@xyz347
Copy link

xyz347 commented Mar 31, 2020

It work, thx.
can you post a new release

nullfy pushed a commit to nullfy/excelize that referenced this issue Oct 23, 2020
nullfy pushed a commit to nullfy/excelize that referenced this issue Oct 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants