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

Incorred relation Id for existing drawings #876

Closed
oldurecu opened this issue Feb 10, 2019 · 3 comments · Fixed by #1462
Closed

Incorred relation Id for existing drawings #876

oldurecu opened this issue Feb 10, 2019 · 3 comments · Fixed by #1462
Labels

Comments

@oldurecu
Copy link

This is:

- [X] a bug report
- [ ] a feature request
- [ ] **not** a usage question (ask them on https://stackoverflow.com/questions/tagged/phpspreadsheet or https://gitter.im/PHPOffice/PhpSpreadsheet)

What is the expected behavior?

Open a file with a drawing in the 3rd sheet. Save it. Open it to view the content

What is the current behavior?

Open it with excel raises a file corrupted error message
Open it with PHPSpreadsheet raises an error "Undefined index rId2"

What are the steps to reproduce?

Here is the analysis
In the original file, the drawing is described by drawing1.xml and has "rId2" has relationship ID
Those data are stored in the $parent->unparsedLoadedData table

When saving, drawing1.xml is renamed drawing3.xml in the ZIP archive, but $unparsedLoadedData is not updated

So when the writer writes the drawing relations (Writer/Xlsx/Rels.php line 214), it looks in $unparsedLoadedData for drawing3.xml, does not find it and uses rId1 as relation ID instead of rId2 that is the original and actual one

I could submit a pull request to fix this, but I don't know the philosophy about handling both unparsedData and data that could be created/added with the library. If I simply update $unparsedLoadedData, I fear to create conflicts between unparsed content and managed content

Let me know how it should work

Which versions of PhpSpreadsheet and PHP are affected?

1.6.0

@xklid101
Copy link
Contributor

xklid101 commented Feb 28, 2019

I can confirm that issue and I found out, that it is caused by adding worksheet before existing worksheet that contains those drawings.

So my temp solution would be to update PhpOffice\PhpSpreadsheet\Spreadsheet::$unparsedLoadedData during adding sheet if needed

According to writer - /xl/drawings/drawing[N].xml files are created by worksheet index in all spreadsheet - so [N] is replaced by current position of particular worksheet - and some other code in writers classes is not compatible with it (as mentioned in comment above)


edit2: (just deleted old code here, because of some problems in some situations)

I've created just simple function to adjust all needed xml content for drawings at once - so it has to be called just before writing - this works for my purposes very well - but I still can not guarantee zero errors on all possibly related code in library
Someone should use it as a temporary solution until library code is really fixed

/**
 * simply loop over all sheets and try to correct drawing reference
 *    should be called just before writing after all actions are completed
 *     
 * @param  Spreadsheet $spreadSheet [description]
 * @return void                   [description]
 */
function adjustUnparsedLoadedData(\PhpOffice\PhpSpreadsheet\Spreadsheet $spreadSheet) {
    if($unparsedLoadedData = $spreadSheet->getUnparsedLoadedData()) {
        $unparsedLoadedDataDoUpdate = false;
        foreach ($spreadSheet->getWorksheetIterator() as $sheet) {
            if(isset($unparsedLoadedData['sheets'][$sheet->getCodeName()]['drawingOriginalIds'])) {
                foreach ($unparsedLoadedData['sheets'][$sheet->getCodeName()]['drawingOriginalIds'] as $key => $value) {
                    if(preg_match('#/drawings/drawing([0-9]+)\.xml$#', $key, $m)) {
                        $drawingNameIndexActual = (int) $m[1];
                        $drawingNameIndexCorrect = ($spreadSheet->getIndex($sheet) + 1);
                        if($drawingNameIndexActual !== $drawingNameIndexCorrect) {

                            // set new correct key => value
                            $unparsedLoadedData
                                ['sheets']
                                [$sheet->getCodeName()]
                                ['drawingOriginalIds']
                                ['../drawings/drawing' . $drawingNameIndexCorrect . '.xml'] = $value;

                            // unset incorrect key => value
                            unset($unparsedLoadedData['sheets'][$sheet->getCodeName()]['drawingOriginalIds'][$key]);
                            $unparsedLoadedDataDoUpdate = true;
                        }
                    }
                }
            }
        }
        if($unparsedLoadedDataDoUpdate)
            $spreadSheet->setUnparsedLoadedData($unparsedLoadedData);
    }
}

@stale
Copy link

stale bot commented Apr 29, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If this is still an issue for you, please try to help by debugging it further and sharing your results.
Thank you for your contributions.

@stale stale bot added the stale label Apr 29, 2019
@stale stale bot closed this as completed May 6, 2019
@maximilianthomas
Copy link

I can confirm both the problem and the proposed solution. Thank you!
Should be re-opened and included in the next release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

3 participants