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

GetPictureCells and GetPictures functions don't support images with absolute path targets #1987

Closed
slashdotdash opened this issue Sep 3, 2024 · 2 comments · Fixed by #1988
Labels
bug Something isn't working

Comments

@slashdotdash
Copy link
Contributor

slashdotdash commented Sep 3, 2024

Description

The GetPictureCells and GetPictures functions don't appear to support images where the target attribute in the XML structure uses absolute paths.

For example the following xl/drawings/_rels/drawing1.xml.rels file which contains a Relationship with a Target attribute using an absolute path of "/xl/media/image1.jpg" does not work.

<?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/image"
        Target="/xl/media/image1.jpg" />
</Relationships>

When the Target attribute is using a relative path of "../media/image1.jpg" it does work.

<?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/image"
        Target="../media/image1.jpg" />
</Relationships>

Steps to reproduce the issue:

Use the attached TestGetPictureAbsolutePath.xls file which contains a single floating image and the following Go test which demonstrates a failure to detect any images when using GetPictureCells.

func TestGetPictureAbs(t *testing.T) {
	f, err := OpenFile(filepath.Join("test", "TestGetPictureAbsolutePath.xlsx"))
	assert.NoError(t, err)

	cells, err := f.GetPictureCells("Sheet1")
	assert.NoError(t, err)
	assert.Equal(t, []string{"A1"}, cells)
	assert.NoError(t, f.Close())

	pics, err := f.GetPictures("Sheet1", "A1")
	assert.NoError(t, err)
	assert.Len(t, pics, 1)
}

Describe the results you received:

The above test fails:

$ go test -run TestGetPictureAbs
--- FAIL: TestGetPictureAbs (0.00s)
    picture_test.go:26: 
        	Error Trace:	picture_test.go:26
        	Error:      	Not equal: 
        	            	expected: []string{"A1"}
        	            	actual  : []string(nil)        	            
        	            	 
        	Test:       	TestGetPictureAbs
FAIL
exit status 1
FAIL	github.com/xuri/excelize/v2	0.300s

Describe the results you expected:

Using either relative or absolute paths for relationship targets should be acceptable.

Output of go version:

go version go1.21.6 darwin/arm64

Excelize version or commit ID:

excelize master @ 0447cb2

Environment details (OS, Microsoft Excel™ version, physical, etc.):

  • macOS 14.6.1
  • Microsoft Excel for Mac version 16.88
@slashdotdash
Copy link
Contributor Author

slashdotdash commented Sep 3, 2024

Note I have been able to resolve the issue locally via a fork of the repo. Please see commit slashdotdash@801f110

I'm happy to submit a PR with the above changes however I am not sure whether they follow the preferred Go or project coding conventions. I also wasn't sure how best to include a test for the changes, other than by adding the test Excel file attached to this issue.

@xuri xuri added the bug Something isn't working label Sep 4, 2024
@xuri xuri moved this to Bugfix in Excelize v2.9.0 Sep 4, 2024
@xuri
Copy link
Member

xuri commented Sep 4, 2024

Thanks for your issue. Your fix is fine, I'll certainly accept that. Note that please do not add .xlsx file, test it after the picture_test.go#L221 like this:

diff --git a/picture_test.go b/picture_test.go
index aec0b5c..9da63f9 100644
--- a/picture_test.go
+++ b/picture_test.go
@@ -219,6 +219,19 @@ func TestGetPicture(t *testing.T) {
    cells, err := f.GetPictureCells("Sheet2")
    assert.NoError(t, err)
    assert.Equal(t, []string{"K16"}, cells)
+
+   // Try to get picture cells with absolute target path in the drawing relationship
+   rels, err := f.relsReader("xl/drawings/_rels/drawing2.xml.rels")
+   assert.NoError(t, err)
+   rels.Relationships[0].Target = "/xl/media/image2.jpeg"
+   cells, err = f.GetPictureCells("Sheet2")
+   assert.NoError(t, err)
+   assert.Equal(t, []string{"K16"}, cells)
+   // Try to get pictures with absolute target path in the drawing relationship
+   pics, err = f.GetPictures("Sheet2", "K16")
+   assert.NoError(t, err)
+   assert.Len(t, pics, 1)
+
    assert.NoError(t, f.Close())
 
    // Test get picture from none drawing worksheet

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Status: Bugfix
Development

Successfully merging a pull request may close this issue.

2 participants