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

Support for LegacyDrawing #1770

Closed
tchari opened this issue Dec 31, 2020 · 0 comments · Fixed by #3130
Closed

Support for LegacyDrawing #1770

tchari opened this issue Dec 31, 2020 · 0 comments · Fixed by #3130

Comments

@tchari
Copy link

tchari commented Dec 31, 2020

This is:

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

I'm using Excel 2019, php 7.4, and the master branch of PhpSpreadsheet

It seems as though the Xlsx Reader does not read the legacyDrawing element. If you read in an xlsm that contains a FormControl button and then save the spreadsheet, the button will be missing when you open the newly saved file. If you unzip the newly saved file, add back the missing legacyDrawing element, and re-zip then the button will be available.

What is the expected behavior?

The saved xlsm retains the button

What is the current behavior?

The saved xlsm no longer has the button

What are the steps to reproduce?

  1. Open Excel 2019
  2. Create a blank workbook
  3. Add a button -> click the Developer tab, then Insert, then select the box icon under Form Controls
  4. In the AssignMacro window, record a new marco. Use the default values. The recorded macro can be whatever you want. I just clicked a random cell and gave it a fixed value (e.g. the character 'a').
  5. Save the file anywhere you want and call it 'test.xlsm'.
  6. Run the code below:
<?php

require __DIR__ . '/vendor/autoload.php';
$reader = new XlsxReader();
$sheet = $reader->load('path/to/test.xlsm');
$writer = new XlsxWriter($sheet);
$writer->save('path/to/test_2.xlsm');
  1. Open 'test_resaved.xlsm' and you'll find there is no button.

Optional

  1. Unpack both test.xlsm and test_resaved.xlsm using your favorite unzip program (I used WinRAR)
  2. Open and compare path/to/test/xl/worksheet/sheet1.xml and path/to/test_resaved/xl/worksheet/sheet1.xml
  3. Somewhere in the file you'll find an element called <drawing> and they will be the same in both files
  4. In the test version of sheet1.xml you'll also find a <legacyDrawing> element. That element will be missing from the test_resaved version.
  5. Copy-paste the legacy drawing element
  6. Recreate test_resaved.xlsm by zipping everything up again (I used WinRAR, fastest)
  7. Open test_resaved.xlsm and you see the button.
@oleibman oleibman mentioned this issue Dec 3, 2021
oleibman added a commit to oleibman/PhpSpreadsheet that referenced this issue Dec 21, 2021
Fix PHPOffice#2396. Fix PHPOffice#1770.

I am starting this out in draft status, and will probably leave it that way for some time. I'm not sure where we want to go with this. It fixes some problems, but in a limited manner, and creates some others. I'm not sure the pain of the others is balanced considering the limitations of the fix. If enough interest is generated as a result of this ticket being out there, we can proceed; if not, it probably isn't worth it.

This fix allows form control elements to be read in and written out. It does not allow you to add such elements, nor even to locate them or determine their properties (so you can't modify or delete them). Although it handles reading and writing of sheets containing both form controls and comments, it will probably create a corrupt spreadsheet if you try adding a new comment to a sheet with form controls - probably quite difficult to solve. Cloning the sheet probably won't work either - probably easier than the other. It is conceivable that we want to add a new property to the Xlsx Reader which turns the reading of form elements on or off (default=off), so that negative effects will be limited to those who have explictly opted in. The change in its current form does not implement such a property.

Because of its limitations, the change isn't really testable. As in some other recent installs, I have added a sample to demonstrate that it works correctly.
oleibman added a commit to oleibman/PhpSpreadsheet that referenced this issue Oct 20, 2022
This is a replacement for draft PR PHPOffice#2455 and draft PR PHPOffice#3127. There is some useful commentary in those PRs which I have mostly, but not entirely, duplicated below. Fix PHPOffice#2396. Fix PHPOffice#1770. Fix PHPOffice#2388.

A related problem is that the vml files used for the form controls sometimes contain invalid xml. Fix PHPOffice#3125 (rejected previous PR PHPOffice#1181 as too risky, issue was also reported as PHPOffice#170). Vml file should be valid Xml, but Excel can generate unclosed `<br>` tags, preventing Xlsx reader from reading file correctly. I believe a very narrowly targeted fix, changing `<br>` to `<br/>`, and only when reading vml files, probably mitigates the risk. The sample file formscomments.xlsx which is part of this change shows this problem with `Button 1` on sheet `Forms`; the spreadsheet was created with Excel 365, so the problem is not restricted to Excel 2013 as originally reported. A comment on PR 3127 indicates that other tags might be involved, but, without a file demonstrating that, I will restrict this change to br tags for now.

I am starting this out in draft status, and will probably leave it that way for some time. I'm not sure where we want to go with this. It fixes some problems, but in a limited manner, and creates some others. I'm not sure the pain of the others is balanced considering the limitations of the fix. If enough interest is generated as a result of this ticket being out there, we can proceed; if not, it probably isn't worth it.

This fix allows form control elements to be read in and written out. It does not allow you to add such elements, nor even to locate them or determine their properties (so you can't modify or delete them). Although it handles reading and writing of sheets containing both form controls and comments, it will probably create a corrupt spreadsheet if you try adding a new comment to a sheet with form controls - probably quite difficult to solve. Cloning the sheet probably won't work either - probably easier than the other. It is conceivable that we want to add a new property to the Xlsx Reader which turns the reading of form elements on or off (default=off), so that negative effects will be limited to those who have explictly opted in. The change in its current form does not implement such a property.

Because of its limitations, the change isn't really testable. As in some other recent installs, I have added a sample to demonstrate that it works correctly.

As it turns out, if we have a worksheet which contains both form controls and comments (see formscomments.xlsx which is part of this PR), PhpSpreadsheet already creates a corrupt file when it tries to load and save the spreadsheet with such a worksheet. With this change, the file is saved without corruption. This tilts things in favor of proceeding. I'm still not ready, but this will be an important consideration.

A sample file for issue PHPOffice#2621 illustrated a problem with shape files. Since they are involved here, I took a look at how the sample worked with this code. In master, and with this change, a corrupt file results. Fixing that is probably easier than the general problem of handling shape files, but it's an argument against moving this forward until the corruption problem can be addressed.

Fix PHPOffice#2661. A template including checkboxes was leading to file corruption solved by this PR. Another argument for moving forward.
oleibman added a commit that referenced this issue Dec 28, 2022
* WIP Limited Support for Form Controls V2 (ListBox, Buttons, etc.)

This is a replacement for draft PR #2455 and draft PR #3127. There is some useful commentary in those PRs which I have mostly, but not entirely, duplicated below. Fix #2396. Fix #1770. Fix #2388.

A related problem is that the vml files used for the form controls sometimes contain invalid xml. Fix #3125 (rejected previous PR #1181 as too risky, issue was also reported as #170). Vml file should be valid Xml, but Excel can generate unclosed `<br>` tags, preventing Xlsx reader from reading file correctly. I believe a very narrowly targeted fix, changing `<br>` to `<br/>`, and only when reading vml files, probably mitigates the risk. The sample file formscomments.xlsx which is part of this change shows this problem with `Button 1` on sheet `Forms`; the spreadsheet was created with Excel 365, so the problem is not restricted to Excel 2013 as originally reported. A comment on PR 3127 indicates that other tags might be involved, but, without a file demonstrating that, I will restrict this change to br tags for now.

I am starting this out in draft status, and will probably leave it that way for some time. I'm not sure where we want to go with this. It fixes some problems, but in a limited manner, and creates some others. I'm not sure the pain of the others is balanced considering the limitations of the fix. If enough interest is generated as a result of this ticket being out there, we can proceed; if not, it probably isn't worth it.

This fix allows form control elements to be read in and written out. It does not allow you to add such elements, nor even to locate them or determine their properties (so you can't modify or delete them). Although it handles reading and writing of sheets containing both form controls and comments, it will probably create a corrupt spreadsheet if you try adding a new comment to a sheet with form controls - probably quite difficult to solve. Cloning the sheet probably won't work either - probably easier than the other. It is conceivable that we want to add a new property to the Xlsx Reader which turns the reading of form elements on or off (default=off), so that negative effects will be limited to those who have explictly opted in. The change in its current form does not implement such a property.

Because of its limitations, the change isn't really testable. As in some other recent installs, I have added a sample to demonstrate that it works correctly.

As it turns out, if we have a worksheet which contains both form controls and comments (see formscomments.xlsx which is part of this PR), PhpSpreadsheet already creates a corrupt file when it tries to load and save the spreadsheet with such a worksheet. With this change, the file is saved without corruption. This tilts things in favor of proceeding. I'm still not ready, but this will be an important consideration.

A sample file for issue #2621 illustrated a problem with shape files. Since they are involved here, I took a look at how the sample worked with this code. In master, and with this change, a corrupt file results. Fixing that is probably easier than the general problem of handling shape files, but it's an argument against moving this forward until the corruption problem can be addressed.

Fix #2661. A template including checkboxes was leading to file corruption solved by this PR. Another argument for moving forward.

* Improved Sample File, and Documentation

Add more realistic worksheet to spreadsheet. Document new feature, adding caveats to how it can be used.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
1 participant