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

binding an internal table with field catalog produces corrupt excel file #474

Closed
plato79 opened this issue Feb 6, 2017 · 7 comments
Closed
Labels

Comments

@plato79
Copy link

plato79 commented Feb 6, 2017

test.xlsx

Attached file is generated via "Save to Frontend" functionality. I'm using SapGui 7.3 with Excel 2013..

In my code first I prepare the internal table, then create field catalog with styles..

After that I just run it like this:

CREATE OBJECT lo_excel TYPE zcl_excel.
*  lo_excel->set_active_sheet_index('1').
  lo_worksheet = lo_excel->get_active_worksheet( ).
  lo_worksheet->set_title( 'Depreciation Master Integration' ).

*  ls_table_settings-table_style  = 'TableStyleMedium13'."zcl_excel_table=>builtinstyle_medium5.
  ls_table_settings-show_row_stripes = abap_true.

  lt_field_catalog = zcl_excel_common=>get_fieldcatalog( ip_table = <fs_t_data> ).

  lo_worksheet->zif_excel_sheet_protection~password   = zcl_excel_common=>encrypt_password( 'sap' ).
  lo_worksheet->zif_excel_sheet_protection~protected  = zif_excel_sheet_protection=>c_protected.
  lo_worksheet->zif_excel_sheet_protection~sheet      = zif_excel_sheet_protection=>c_active.
  lo_worksheet->zif_excel_sheet_protection~objects    = zif_excel_sheet_protection=>c_active.
  lo_worksheet->zif_excel_sheet_protection~scenarios  = zif_excel_sheet_protection=>c_active.

  lo_worksheet->bind_table( ip_table          = <fs_t_data>
                            is_table_settings = ls_table_settings
                            it_field_catalog  = lt_field_catalog
                            iv_default_descr  = 'L' ).

  lo_worksheet->freeze_panes( ip_num_rows = 1 ). "freeze column headers when scrolling

Also, I think I should mention that I use CRLF in headers like this:

<fs_fieldcatalog>-scrtext_l = |Input{ cl_abap_char_utilities=>cr_lf }{ <fs_fieldcatalog>-fieldname+3(4) }-{ <fs_fieldcatalog>-fieldname+7(2) }|.

First, I get this error when opening:

image

If I click yes, I'm not really sure it does anything at all but it says it repaired records like this:

image

... and outputs this log file:

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<recoveryLog xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main"><logFileName>error080680_01.xml</logFileName><summary>Errors were detected in file 'C:\Users\hakatac\AppData\Local\_MovedObjects\Desktop\test.xlsx'</summary><repairedRecords summary="Following is a list of repairs:"><repairedRecord>Repaired Records: Table from /xl/tables/table1.xml part (Table)</repairedRecord></repairedRecords></recoveryLog>

The thing is, Excel doesn't really do anything.. I checked it by opening XLSX file with 7zip and it seems it just removes the CRLF characters from table1.xml file. Well and good, but if I save file like this, and try to reopen I get the same error.

One other thing, If I click diagonal Select marker Excel freezes like this:

image

This is not only in my machine, other people who tested reported same thing.. What could be the issue.

BTW, here is the fixed(!) file:

test - Copy.xlsx

@rmcknight001
Copy link

rmcknight001 commented Feb 10, 2017 via email

@plato79
Copy link
Author

plato79 commented Feb 13, 2017

Tried with only CR like :
<fs_fieldcatalog>-scrtext_l = |Capitalization{ cl_abap_char_utilities=>cr_lf(1) }{ <fs_fieldcatalog>-fieldname+3 }|.

with only LF like:

<fs_fieldcatalog>-scrtext_l = |Capitalization{ cl_abap_char_utilities=>cr_lf+1 }{ <fs_fieldcatalog>-fieldname+3 }|.

same result. Actually only CR made the result very bad...

Removing CRLF altogether fixes the repair process ( no popup or nothing )

But again selecting all fields with selection marker at the corner still freezes Excel. So there is definitely something wrong but not about CRLF character.

Because I use field names to set headers and both of them are dynamic, it'll be too complex to set headers like you explained. While I know that it's not entirely impossible, the biggest problem is about the excel crashing. People want to select the values from this table and paste to another and they cannot do it right now.

I also saw a CPU usage higher than usual... Excel seems to be using at least 25% CPU when crashed. Before crashing it uses only about 1 percent or none at all..

@oliver-huetkoeper
Copy link
Contributor

The line break is not allowed here. It needs to be escaped by ABAP2XLSX when creating the tables XML document.

So it should not be

<tableColumn id="6" name="Capitalization
2016"/>

but
<tableColumn id="6" name="Capitalization_x000a_2016"/>

See: https://msdn.microsoft.com/en-us/library/ff534667(v=office.12).aspx
Looking at the OOXML documentation there are many more cases where the ST_Xstring data type is used and where this needs to be fixed in the writer class. And probably it needs to be fixed in the reader class as well.
Maybe I find the time to have a deeper look at this.

Best regards,
Oliver

@plato79
Copy link
Author

plato79 commented May 16, 2017

While two lined headers are a problem, the other problem I mentioned ( freezing of Excel when selecting all files ) still persists even when I reduce all headers to 1 line.

I also found out, if I open it via Excel and save it, and then when I reopen it, the problem disappears. So it seems there's something fishy going on... Like maybe Excel removes some unneeded but problematic code from XML files or something.

BTW, Excel in Office for Mac doesn't have this problem. But I don't think it counts...

@sandraros
Copy link
Collaborator

I confirm what @oliver-huetkoeper has explained 4 years ago, abap2xlsx should be corrected so that \n (newline character) is replaced with x000a in the "tables XML document":

  • table1.xml contains <tableColumn id="6" name="Capitalization_x000a_2016"/>

and sharedStrings.xml could still contain \n (but it seems that x000a should also work, although Excel saves with \n, see below):

<si><t>Capitalization
2016</t></si>

In OOXML, this is said about name of tableColumn: "A string representing the unique caption of the table column. This is what shall be displayed in the header row in the UI, and is referenced through functions. This name shall be unique per table. The possible values for this attribute are defined by the ST_Xstring simple type (§22.9.2.19)."

§22.9.2.19 = https://msdn.microsoft.com/en-us/library/ff534667(v=office.12).aspx

It's also said that t of sharedStrings is of the same type ST_Xstring. So x000a could also be used.

NB: problem also referred there with minimal reproducible example https://answers.sap.com/questions/13408567/abap2xlsx-excel-xlsx-repair-due-to-new-line-charac.html

sandraros pushed a commit to sandraros/abap2xlsx that referenced this issue Jun 20, 2021
Fix of abap2xlsx#474, \r\n can now be used in field catalog of table header in bind_table to display text on several lines. ZDEMO_EXCEL22 demonstrates it in first column "Flight Number" on 2 lines.
gregorwolf added a commit that referenced this issue Jun 21, 2021
* First solution, but wrong

Excel saves table1.xml with _x000d__x000a_ but abap2xlsx doing the same thing makes Excel say it's wrong!? To be analyzed...

* non-final solution

* Fix newline now accepted in table header

Fix of #474, \r\n can now be used in field catalog of table header in bind_table to display text on several lines. ZDEMO_EXCEL22 demonstrates it in first column "Flight Number" on 2 lines.

Co-authored-by: sandraros <sandra.rossi@gmail.com>
Co-authored-by: Gregor Wolf <gregor.wolf@gmail.com>
@sandraros
Copy link
Collaborator

@plato79 Could you confirm that the fix solves your issue of CRLF in a table heading? (you may run ZDEMO_EXCEL22) If it's fixed for you, please close the issue. Regards.

@sandraros
Copy link
Collaborator

sandraros commented Sep 19, 2021

No answer. Closing.
I see that there was no complete minimal reproducible example provided in this issue. Here it is (can be used in future regression tests).

DATA gc_save_file_name TYPE string VALUE 'bug_474.xlsx'.
INCLUDE zdemo_excel_outputopt_incl.
START-OF-SELECTION.
  TRY.
      DATA(excel) = NEW zcl_excel( ).
      SELECT * FROM sbook UP TO 10 ROWS INTO TABLE @DATA(gt_sbook).
      DATA(field_catalog) = zcl_excel_common=>get_fieldcatalog( ip_table = gt_sbook ).
      field_catalog[ 2 ]-scrtext_l = |Header on{ cl_abap_char_utilities=>cr_lf }two rows|.
      field_catalog[ 3 ]-scrtext_l = |Header\twith\tthree\ttabs|.
      excel->get_active_worksheet( )->bind_table( ip_table          = gt_sbook
                                                  it_field_catalog  = field_catalog
                                                  iv_default_descr  = 'L' ).
      lcl_output=>output( excel ).
    CATCH cx_root INTO DATA(lx_root).
      MESSAGE lx_root TYPE 'I' DISPLAY LIKE 'E'.
  ENDTRY.

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

No branches or pull requests

4 participants