Problem accessing comments background images (Using Excel generated via Flexcel for .net)

[Kashorako]

This is:

- [*] 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?

We should receive all the workbook comments and its background images

What is the current behavior?

When we select a Excel file (generated via Flexcel for .net), when we try obtain the comments' background images, we receive an empty array in the drawing object

What are the steps to reproduce?

We are using Laravel 7, we select the Excel file (generated via Flexcel for .net) and when we try to access the comments in order to get its background image, it returns empty, IT WORKS normally when using a normal Excel (not generated via Flexcel for .net)

In the code below, $file is the uploaded Excel file

<?php

require __DIR__ . '/vendor/autoload.php';

// Create new Spreadsheet object
$reader = new \PhpOffice\PhpSpreadsheet\Reader\Xls();
                $reader = $reader->setReadDataOnly(false);
                $sheet = $reader->load($file)->getSheet(0);

$allComments = $sheet->getComments();
              foreach($allComments  as $c => $comment) {
                  if($comment->hasBackgroundImage()){ // return false

                  }
              }

What features do you think are causing the issue

• [*] Reader
• Writer
• Styles
• Data Validations
• Formula Calculations
• Charts
• AutoFilter
• Form Elements

Does an issue affect all spreadsheet file formats? If not, which formats are affected?

It affects all Excel files generated using Flexcel

Which versions of PhpSpreadsheet and PHP are affected?

We are using PhpSpreadsheet 1.28

9_erroneo.xlsx

[oleibman]

Thank you for reporting the problem. The code, in several places, is testing if ($zip->locateName()) for a truth-y value, but the function can actually return 0, as it happens to do with your spreadsheet. They should all be recoded as if ($zip->locateName() !== false). I will have a fix available in the next few days.

[Kashorako]

Thank you so much for your quick answer, we tried out the code of your pull request
#3655
and the problem with the file 9_erroneo.xlsx is now solved, but now when we try to upload the complete file (data.xlsx) it gives us the same error "PhpOffice\PhpSpreadsheet\Exception: Unsupported image type in comment background. Supported types: PNG, JPEG, BMP, GIF"

Should we open another thread in order to get help for this issue?
I'm attaching the complete file which gives us the error mentioned before
Data.xlsx

[oleibman]

I am unable to duplicate your problem with the new spreadsheet; I can load it without any exceptions. Is it possible for you to insert a statement in Worksheet/Drawing so that I can get more information about your problem:

    public function getMediaFilename()
    {
        if (!array_key_exists($this->type, self::IMAGE_TYPES_CONVERTION_MAP)) {
           var_dump($this->type, $this->path); // add this statement
            throw new PhpSpreadsheetException('Unsupported image type in comment background. Supported types: PNG, JPEG, BMP, GIF.');
        }

[Kashorako]

Hi, thanks for your answer.
Can you try again with this new file, (the last one was a wrong one),
Using this new file, once you upload it, you'll receive the exception "PhpOffice\PhpSpreadsheet\Exception: Unsupported image type in comment background. Supported types: PNG, JPEG, BMP, GIF" in "PhpSpreadsheet/Comment.php:343".

If we comment this section

if (!array_key_exists($this->type, self::IMAGE_TYPES_CONVERTION_MAP)) {
            throw new PhpSpreadsheetException('Unsupported image type in comment background. Supported types: PNG, JPEG, BMP, GIF.');
        }

and the code proceeds , the file is uploaded but if we use

$allComments = $sheet->getComments();

getComments returns an empty array, without the Drawing object.
And if you check the new file, there is a lot of background images as comments inside several cells
Data.xlsx

[oleibman]

Thank you. I am able to duplicate your problem with this one. Let me see what I can find out.

[Kashorako]

Thank you so much @oleibman , we will be waiting for any news and please let us know if we can help.

[oleibman]

Please open a new ticket so that I can install the fix for your first problem while continuing to work on your second.

The second problem definitely has to do with the way Flexcel generated the spreadsheet. PhpSpreadsheet is having a difficult time parsing some of the file. There are unexpected problems with how the background images are being referenced. That is actually pretty easy to fix. Unfortunately, while that fix does avoid throwing an exception, the file that it winds up generating is somehow corrupt. I found another problem referencing the printer settings file, and that one does not seem so easy to fix. And, even if I do fix it, I don't know if any other new problems will crop up. Hence my request for a new ticket.

As a temporary workaround which might or might not be useful to you, if you open the Flexcel-generated file in Excel, and "save as" some other file, PhpSpreadsheet has no problem handling the other file.

[oleibman]

Good news - I think I've solved the Flexcel-generated printer settings problem, and there don't appear to be any others. Hold off on opening another ticket for now.

[Kashorako]

Awesome! I was just writing the new ticket, i'll wait for your answer!

[oleibman]

Might it be possible to generate a much smaller version of Data.xlsx (7.8MB) that has the same problem with the current code that I can use for a formal test case?

[Kashorako]

I'm attaching a smaller file, but i'm not sure if the error will be the same
This new file has been shrunk to 50 rows using WPS

The original file (Data.xlsx) has been generated using Polux (a tool used to inspect poles) that tool exports the Excel using Flexcel

We will find a smaller Excel generated via Polux as soon as possible and let you know

Data-corto.xlsx

[oleibman]

Unfortunately, the new file lacks a printer settings file within it, and also references the background images as we expected, so it can't really be used as a test case. I plan to push my latest changes tomorrow; hopefully you'll have a smaller test file available for me by then.

[Kashorako]

I just talked to a person who owns a Polux tool, and he will send me more files generated via Flexcel by Monday, I'll let you know as soon as I have the new files, so we can test again. Thank you so much for your help, I'll let you know any news.

…

On Fri, Jul 28, 2023, 6:29 PM oleibman ***@***.***> wrote: Unfortunately, the new file lacks a printer settings file within it, and also references the background images as we expected, so it can't really be used as a test case. I plan to push my latest changes tomorrow; hopefully you'll have a smaller test file available for me by then. — Reply to this email directly, view it on GitHub <#3654 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AQWKY72S3DJOLPAESWFIGG3XSQ4NBANCNFSM6AAAAAA22BWSFE> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[Kashorako]

Hi @oleibman, our friend sent us a file with 12 rows, it was generated using the Polux tool as well, but it works just fine because it didn't use Flexcel (we don't know why yet), we are trying to obtain another file generated via Polux but using Flexcel, so we can keep testing.

In the meantime, do you think it's possible that you test your commits and changes using the file Data.xlsx (7.8MB)?

Good news - I think I've solved the Flexcel-generated printer settings problem, and there don't appear to be any others. Hold off on opening another ticket for now.

Also, did you manage to solve the Flexcel-generated printer settings problem?

Thank you so much for your help and time

[oleibman]

I've pushed my latest changes, which do solve the printer settings problem. I would still like to have a smaller file to substitute for the large file that is currently part of the PR if at all possible.

[oleibman]

@Kashorako Any progress on finding a smaller file?