Skip to content

Commit

Permalink
[IMP] XLSX: Faulty default styles
Browse files Browse the repository at this point in the history
Due to some mismatch between the style in the default exported construct
and the default values computed for a cell style, we were creating
useless style entries at export.

By setting a style index on a worksheet cell entry, our import code would
then apply this style to the cell even though that same cell, when
exported, had no style whatsoever.

Furthermore, we did not process Excel default color properly.
In Xlsx file, the color is sometimes set specifically to
"FF000000" in the style sheet. However, that color (black) is not
applied as a user-defined color in Excel (i.e. it's importance is lower
than the color applied by a table[^1]). Instead, a user-defined
"black" will be referred to via a theme property[^2].

Since we did not detect this "default color" pattern, we would consider
the attribute `rgb=FF000000` as a user-defined color an ultimately set
it in the plugin structure at import.

This revision detects the "default color" situation and discards the
given color to let the library (and specifically the CellPlugin as that
issue affects the cell font color in particular) use its own default
behaviour.

[^1]: To test this out:
      - Go to Excel and create a sheet on which you put
        a table.
      - Apply a style such that the text turns white
      - On the cells with white text, manually apply an italic format
      - Import the xlsx file inside o-spreadsheet
      ==> the text turned black in o-spreadsheet

[^2]: This situation seems to only concern that specific color and might
encompass a broader scope but I haven't been able to find proper
documentation about this.

closes #3799

Signed-off-by: Rémi Rahir (rar) <[email protected]>
  • Loading branch information
rrahir committed Apr 19, 2024
1 parent cca91dd commit 297841e
Show file tree
Hide file tree
Showing 6 changed files with 1,087 additions and 1,338 deletions.
8 changes: 8 additions & 0 deletions src/xlsx/conversion/color_conversion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,11 @@ export function hexaToInt(hex: Color) {
}
return parseInt(hex.replace("#", ""), 16);
}

/**
* When defining style (fontColor, borderColor for instance)
* Excel will specify rgb="FF000000"
* In that case, We should not consider this value as user-defined but
* rather like an instruction: "Use your system default"
*/
export const DEFAULT_SYSTEM_COLOR = "FF000000";
4 changes: 3 additions & 1 deletion src/xlsx/extraction/base_extractor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
XLSXTheme,
XMLFile,
} from "../../types/xlsx";
import { DEFAULT_SYSTEM_COLOR } from "../conversion";
import { fixXlsxUnicode } from "../helpers/misc";
import { XLSXImportWarningManager } from "../helpers/xlsx_parser_error_manager";
import { escapeQueryNameSpaces } from "../helpers/xml_helpers";
Expand Down Expand Up @@ -327,9 +328,10 @@ export class XlsxBaseExtractor {
rgb = this.getThemeColor(themeIndex, theme.clrScheme);
} else {
rgb = this.extractAttr(colorElement, "rgb")?.asString();
rgb = rgb === DEFAULT_SYSTEM_COLOR ? undefined : rgb;
}
const color = {
rgb,
rgb: rgb || defaultColor,
auto: this.extractAttr(colorElement, "auto")?.asBool(),
indexed: this.extractAttr(colorElement, "indexed")?.asNum(),
tint: this.extractAttr(colorElement, "tint")?.asNum(),
Expand Down
5 changes: 4 additions & 1 deletion src/xlsx/functions/worksheet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,10 @@ export function addRows(

// style
const id = normalizeStyle(construct, extractStyle(cell, data));
attributes.push(["s", id]);
// don't add style if default
if (id) {
attributes.push(["s", id]);
}

let additionalAttrs: XMLAttributes = [];
let cellNode = escapeXml``;
Expand Down
2 changes: 1 addition & 1 deletion src/xlsx/helpers/content_helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export function extractStyle(cell: ExcelCellData, data: WorkbookData): Extracted
vertical: style.verticalAlign
? V_ALIGNMENT_EXPORT_CONVERSION_MAP[style.verticalAlign]
: undefined,
wrapText: style.wrapping === "wrap",
wrapText: style.wrapping === "wrap" || undefined,
},
};

Expand Down
4 changes: 2 additions & 2 deletions src/xlsx/helpers/xml_helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,15 +93,15 @@ export function getDefaultXLSXStructure(data: ExcelWorkbookData): XLSXStructure
fillId: 0,
numFmtId: 0,
borderId: 0,
alignment: { vertical: "bottom" },
alignment: {},
},
],
fonts: [
{
size: DEFAULT_FONT_SIZE,
family: 2,
color: { rgb: "000000" },
name: "Calibri",
name: "Arial",
},
],
fills: [{ reservedAttribute: "none" }, { reservedAttribute: "gray125" }],
Expand Down
Loading

0 comments on commit 297841e

Please sign in to comment.