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

Fix anchor.js #892

Merged
merged 1 commit into from
Oct 18, 2019
Merged

Fix anchor.js #892

merged 1 commit into from
Oct 18, 2019

Conversation

wwojtkowski
Copy link
Contributor

It doesn't work when you have the address with nativeCol = 0

It doesn't work when you have the address with nativeCol = 0
@Siemienik
Copy link
Member

Col and row are always >= 1

But rigth, that's good pull request

@Siemienik
Copy link
Member

@wwojtkowski could i please you for write a test that's failed this code before changes?

@wwojtkowski
Copy link
Contributor Author

Col and row are always >= 1

But rigth, that's good pull request

I cannot agree with you. Rows are always >= 1 but not col.
I have Excel's file where I have images in the first column in every row.
In the Anchor class nativeCol has 0 value but nativeRow has proper value (>=1).
Before my fix, all images from worksheet have nativeRow = 0 and nativeCol = 0 - which is a very serious bug.
Zrzut ekranu 2019-07-13 o 18 27 15

@Siemienik
Copy link
Member

@wwojtkowski ok, thanks for the explanation :)

I want to merge it, but it's required to cover changes by tests

@vidyakin
Copy link

@Siemienik , I have error with the word 'anchor' when call openFile, may it be the same problem? :)

TypeError: Cannot read property 'anchors' of undefined
    at DrawingXform.reconcile (C:\web\my_prj\node_modules\exceljs\lib\xlsx\xform\drawing\drawing-xform.js:94:11)
    at Object.keys.forEach.name (C:\web\my_prj\node_modules\exceljs\lib\xlsx\xlsx.js:103:22)
    at Array.forEach (<anonymous>)
    at XLSX.reconcile (C:\web\my_prj\node_modules\exceljs\lib\xlsx\xlsx.js:95:33)
    at PromiseLib.Promise.all.then (C:\web\my_prj\node_modules\exceljs\lib\xlsx\xlsx.js:389:16)

But it appears in the ONLY Excel file that is important for my task!
And there are some interesting things I found:

  • If I copy all worksheet and paste to new workbook, it works.
  • If I run in Excel "check compatibility" function and do correct operation - it works!

@guyonroche guyonroche merged commit f7c5e1f into exceljs:master Oct 18, 2019
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

Successfully merging this pull request may close these issues.

None yet

4 participants