Skip to content

Commit 8da3543

Browse files
FrancescoMolinaroAndrea Barbasso
FrancescoMolinaro
authored and
Andrea Barbasso
committed
Merged in task/dspace-cris-2024_02_x/DSC-2811 (pull request DSpace#4335)
[DSC-2811] fix issue with image exports, add tests Approved-by: Andrea Barbasso
2 parents 9eb3903 + 74b7773 commit 8da3543

12 files changed

Lines changed: 312 additions & 22 deletions

File tree

‎src/app/core/export-service/browser-export.service.spec.ts‎

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
/* eslint-disable import/no-namespace */
2+
import * as fileSaver from 'file-saver';
3+
import { ExportAsService } from 'ngx-export-as';
4+
import {
5+
BehaviorSubject,
6+
of,
7+
} from 'rxjs';
8+
9+
import { BrowserExportService } from './browser-export.service';
10+
import { ExportImageType } from './export.service';
11+
12+
describe('BrowserExportService', () => {
13+
let service: BrowserExportService;
14+
let mockDocument: any;
15+
let mockWindow: any;
16+
17+
beforeEach(() => {
18+
mockWindow = {
19+
nativeWindow: {
20+
location: {
21+
origin: 'http://localhost:9876',
22+
},
23+
},
24+
};
25+
26+
mockDocument = {
27+
styleSheets: [],
28+
};
29+
30+
service = new BrowserExportService('browser', mockWindow, mockDocument);
31+
});
32+
33+
describe('exportAsFile', () => {
34+
it('should create ExportAsService and call save', () => {
35+
const mockResult = of(null);
36+
spyOn(ExportAsService.prototype, 'save').and.returnValue(mockResult as any);
37+
38+
const result = service.exportAsFile('csv', 'element-id', 'test-file');
39+
40+
expect(ExportAsService.prototype.save).toHaveBeenCalled();
41+
expect(service.exportAsConfig).toEqual(jasmine.objectContaining({
42+
type: 'csv',
43+
elementIdOrContent: 'element-id',
44+
fileName: 'test-file',
45+
download: true,
46+
}));
47+
expect(result).toBe(mockResult);
48+
});
49+
50+
it('should pass download=false when specified', () => {
51+
spyOn(ExportAsService.prototype, 'save').and.returnValue(of(null) as any);
52+
53+
service.exportAsFile('xlsx', 'element-id', 'test-file', false);
54+
55+
expect(service.exportAsConfig.download).toBe(false);
56+
});
57+
});
58+
59+
describe('exportAsImage', () => {
60+
let domNode: HTMLElement;
61+
let isLoading: BehaviorSubject<boolean>;
62+
63+
beforeEach(() => {
64+
domNode = document.createElement('div');
65+
isLoading = new BehaviorSubject<boolean>(true);
66+
});
67+
68+
it('should call collectSameOriginFontCSS and set isLoading to false after export for png', async () => {
69+
spyOn(service, 'collectSameOriginFontCSS').and.returnValue('');
70+
71+
service.exportAsImage(domNode, ExportImageType.png, 'test-image', isLoading);
72+
await new Promise(resolve => setTimeout(resolve, 100));
73+
74+
expect(service.collectSameOriginFontCSS).toHaveBeenCalled();
75+
expect(isLoading.getValue()).toBe(false);
76+
});
77+
78+
it('should call collectSameOriginFontCSS and set isLoading to false after export for jpeg', async () => {
79+
spyOn(service, 'collectSameOriginFontCSS').and.returnValue('');
80+
81+
service.exportAsImage(domNode, ExportImageType.jpeg, 'test-image', isLoading);
82+
await new Promise(resolve => setTimeout(resolve, 100));
83+
84+
expect(service.collectSameOriginFontCSS).toHaveBeenCalled();
85+
expect(isLoading.getValue()).toBe(false);
86+
});
87+
88+
it('should pass fontEmbedCSS from collectSameOriginFontCSS to the export', async () => {
89+
const fakeFontCSS = '@font-face { font-family: Test; }';
90+
spyOn(service, 'collectSameOriginFontCSS').and.returnValue(fakeFontCSS);
91+
92+
service.exportAsImage(domNode, ExportImageType.png, 'test', isLoading);
93+
await new Promise(resolve => setTimeout(resolve, 100));
94+
95+
expect(service.collectSameOriginFontCSS).toHaveBeenCalled();
96+
});
97+
});
98+
99+
describe('exportImageWithBase64', () => {
100+
let isLoading: BehaviorSubject<boolean>;
101+
102+
beforeEach(() => {
103+
isLoading = new BehaviorSubject<boolean>(true);
104+
});
105+
106+
it('should call saveAs with the base64 string when it has a value', () => {
107+
spyOn(fileSaver, 'saveAs');
108+
const base64 = 'data:image/png;base64,abc123';
109+
110+
service.exportImageWithBase64(base64, ExportImageType.png, 'my-file', isLoading);
111+
112+
expect(fileSaver.saveAs).toHaveBeenCalledWith(base64, 'my-file.png');
113+
expect(isLoading.getValue()).toBe(false);
114+
});
115+
116+
it('should not call saveAs and should log error when base64 is null', () => {
117+
spyOn(fileSaver, 'saveAs');
118+
spyOn(console, 'error');
119+
120+
service.exportImageWithBase64(null, ExportImageType.jpeg, 'my-file', isLoading);
121+
122+
expect(fileSaver.saveAs).not.toHaveBeenCalled();
123+
expect(console.error).toHaveBeenCalledWith('Base64 string is empty');
124+
expect(isLoading.getValue()).toBe(false);
125+
});
126+
127+
it('should set isLoading to false regardless', () => {
128+
spyOn(fileSaver, 'saveAs');
129+
130+
service.exportImageWithBase64('data:image/jpeg;base64,xyz', ExportImageType.jpeg, 'file', isLoading);
131+
132+
expect(isLoading.getValue()).toBe(false);
133+
});
134+
});
135+
136+
describe('collectSameOriginFontCSS', () => {
137+
it('should skip cross-origin stylesheets', () => {
138+
const crossOriginSheet = { href: 'https://www.gstatic.com/charts/50/css/core/tooltip.css', cssRules: [] };
139+
mockDocument.styleSheets = [crossOriginSheet];
140+
141+
const fontCSS = service.collectSameOriginFontCSS();
142+
143+
expect(fontCSS).toBe('');
144+
});
145+
146+
it('should silently skip stylesheets that throw on cssRules access', () => {
147+
spyOn(console, 'error');
148+
149+
const badSheet = {
150+
href: null,
151+
get cssRules(): CSSRuleList {
152+
throw new DOMException('Not allowed');
153+
},
154+
};
155+
mockDocument.styleSheets = [badSheet];
156+
157+
const fontCSS = service.collectSameOriginFontCSS();
158+
159+
expect(console.error).toHaveBeenCalledWith('Error appending sheet to export: ', null);
160+
expect(fontCSS).toBe('');
161+
});
162+
163+
it('should collect @font-face rules from null-href (inline) stylesheets', () => {
164+
const fontRule1 = { constructor: { name: 'CSSFontFaceRule' }, cssText: '@font-face { font-family: A; }' };
165+
const fontRule2 = { constructor: { name: 'CSSFontFaceRule' }, cssText: '@font-face { font-family: B; }' };
166+
const nonFontRule = { constructor: { name: 'CSSStyleRule' }, cssText: 'body { margin: 0; }' };
167+
mockDocument.styleSheets = [{ href: null, cssRules: [fontRule1, nonFontRule, fontRule2] }];
168+
169+
const fontCSS = service.collectSameOriginFontCSS();
170+
171+
expect(fontCSS).toBe('@font-face { font-family: A; }\n@font-face { font-family: B; }');
172+
});
173+
});
174+
});
175+

‎src/app/core/export-service/browser-export.service.ts‎

Lines changed: 55 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { DOCUMENT } from '@angular/common';
12
import {
23
Inject,
34
PLATFORM_ID,
@@ -15,6 +16,10 @@ import {
1516
import { BehaviorSubject } from 'rxjs';
1617

1718
import { hasValue } from '../../shared/empty.util';
19+
import {
20+
NativeWindowRef,
21+
NativeWindowService,
22+
} from '../services/window.service';
1823
import {
1924
ExportImageType,
2025
ExportService,
@@ -33,7 +38,9 @@ export class BrowserExportService implements ExportService {
3338
*/
3439
exportAsConfig: ExportAsConfig;
3540

36-
constructor(@Inject(PLATFORM_ID) private platformId: any) {
41+
constructor(@Inject(PLATFORM_ID) private platformId: any,
42+
@Inject(NativeWindowService) private _window: NativeWindowRef,
43+
@Inject(DOCUMENT) private document: any) {
3744
}
3845

3946
/**
@@ -66,25 +73,59 @@ export class BrowserExportService implements ExportService {
6673
* @param isLoading A boolean representing the exporting process status.
6774
*/
6875
exportAsImage(domNode: HTMLElement, type: ExportImageType, fileName: string, isLoading: BehaviorSubject<boolean>): void {
76+
// html-to-image internally iterates ALL document.styleSheets to inline fonts.
77+
// Cross-origin sheets (e.g. Google Charts CSS from gstatic.com) throw a DOMException
78+
// in Firefox when .cssRules is accessed (Same-Origin Policy).
79+
const fontEmbedCSS = this.collectSameOriginFontCSS();
6980

70-
const options: Options = { backgroundColor: '#ffffff' };
81+
const options: Options = {
82+
backgroundColor: '#ffffff',
83+
fontEmbedCSS,
84+
};
7185

72-
if (type === ExportImageType.png) {
73-
toPng(domNode, options)
74-
.then((dataUrl) => {
75-
saveAs(dataUrl, fileName + '.' + type);
76-
isLoading.next(false);
77-
});
78-
} else {
79-
toJpeg(domNode, options)
80-
.then((dataUrl) => {
81-
saveAs(dataUrl, fileName + '.' + type);
82-
isLoading.next(false);
83-
});
86+
const export$ = type === ExportImageType.png
87+
? toPng(domNode, options)
88+
: toJpeg(domNode, options);
89+
90+
export$.then((dataUrl) => {
91+
saveAs(dataUrl, fileName + '.' + type);
92+
isLoading.next(false);
93+
}).catch((err) => {
94+
console.error('Image export failed', err);
95+
isLoading.next(false);
96+
});
97+
}
98+
99+
/**
100+
* Collects all @font-face CSS rules from same-origin stylesheets only.
101+
* Cross-origin sheets are silently skipped so they never trigger a
102+
* DOMException when cssRules is accessed in Firefox.
103+
*/
104+
collectSameOriginFontCSS(): string {
105+
const fontRules: string[] = [];
106+
const sheets = Array.from((this.document as Document).styleSheets);
107+
108+
for (const sheet of sheets) {
109+
// Skip cross-origin sheets — accessing cssRules on them throws in Firefox
110+
if (sheet.href && new URL(sheet.href).origin !== this._window.nativeWindow.location.origin) {
111+
continue;
112+
}
113+
try {
114+
const rules = Array.from(sheet.cssRules ?? []);
115+
for (const rule of rules) {
116+
if (rule.constructor.name === 'CSSFontFaceRule') {
117+
fontRules.push(rule.cssText);
118+
}
119+
}
120+
} catch {
121+
console.error('Error appending sheet to export: ', sheet.href);
122+
}
84123
}
85124

125+
return fontRules.join('\n');
86126
}
87127

128+
88129
/**
89130
* Creates an image from the given base64 string.
90131
* @param base64 the base64 string

‎src/app/statistics-page/cris-statistics-page/statistics-chart/statistics-chart-bar/statistics-chart-bar.component.spec.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
import { CommonModule } from '@angular/common';
1+
import {
2+
CommonModule,
3+
DOCUMENT,
4+
} from '@angular/common';
25
import {
36
DebugElement,
47
NO_ERRORS_SCHEMA,
@@ -13,6 +16,10 @@ import { NoopAnimationsModule } from '@angular/platform-browser/animations';
1316
import { TranslateModule } from '@ngx-translate/core';
1417
import { ChartComponent } from '@swimlane/ngx-charts';
1518
import { of as observableOf } from 'rxjs';
19+
import {
20+
NativeWindowRef,
21+
NativeWindowService,
22+
} from 'src/app/core/services/window.service';
1623

1724
import { BrowserExportService } from '../../../../core/export-service/browser-export.service';
1825
import { REPORT_DATA } from '../../../../core/statistics/data-report.service';
@@ -76,6 +83,8 @@ describe('StatisticsChartBarComponent', () => {
7683
{ provide: REPORT_DATA, useValue: selectedReport },
7784
{ provide: BrowserExportService, useValue: exportServiceStub },
7885
{ provide: 'categoryType', useValue: 'mainReports' },
86+
{ provide: NativeWindowService, useValue: new NativeWindowRef() },
87+
{ provide: DOCUMENT, useValue: document },
7988
],
8089
schemas: [NO_ERRORS_SCHEMA],
8190
}).overrideComponent(StatisticsChartBarComponent, { remove: { imports: [ChartComponent, AlertComponent] } }).compileComponents();

‎src/app/statistics-page/cris-statistics-page/statistics-chart/statistics-chart-bar/statistics-chart-bar.component.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import {
22
AsyncPipe,
3+
DOCUMENT,
34
NgIf,
45
} from '@angular/common';
56
import {
@@ -16,6 +17,10 @@ import {
1617
import { ChartComponent } from '../../../../charts/components/chart/chart.component';
1718
import { ChartData } from '../../../../charts/models/chart-data';
1819
import { ChartSeries } from '../../../../charts/models/chart-series';
20+
import {
21+
NativeWindowRef,
22+
NativeWindowService,
23+
} from '../../../../core/services/window.service';
1924
import { REPORT_DATA } from '../../../../core/statistics/data-report.service';
2025
import {
2126
Point,
@@ -54,8 +59,10 @@ export class StatisticsChartBarComponent extends StatisticsChartDataComponent {
5459
@Inject(REPORT_DATA) public report: UsageReport,
5560
@Inject('categoryType') public categoryType: string,
5661
@Inject(PLATFORM_ID) protected platformId: any,
62+
@Inject(NativeWindowService) protected _window: NativeWindowRef,
63+
@Inject(DOCUMENT) protected document: any,
5764
) {
58-
super(report, categoryType, platformId);
65+
super(report, categoryType, platformId, _window, document);
5966
}
6067

6168
/**

‎src/app/statistics-page/cris-statistics-page/statistics-chart/statistics-chart-data/statistics-chart-data.component.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
import { isPlatformBrowser } from '@angular/common';
1+
import {
2+
DOCUMENT,
3+
isPlatformBrowser,
4+
} from '@angular/common';
25
import {
36
Component,
47
ElementRef,
@@ -20,6 +23,10 @@ import {
2023
ExportImageType,
2124
ExportService,
2225
} from '../../../../core/export-service/export.service';
26+
import {
27+
NativeWindowRef,
28+
NativeWindowService,
29+
} from '../../../../core/services/window.service';
2330
import { REPORT_DATA } from '../../../../core/statistics/data-report.service';
2431
import {
2532
Point,
@@ -67,14 +74,16 @@ export abstract class StatisticsChartDataComponent implements OnInit {
6774
@Inject(REPORT_DATA) public report: UsageReport,
6875
@Inject('categoryType') public categoryType: string,
6976
@Inject(PLATFORM_ID) protected platformId: any,
77+
@Inject(NativeWindowService) protected _window: NativeWindowRef,
78+
@Inject(DOCUMENT) protected document: any,
7079
) {
7180

7281
/* IMPORTANT
7382
Due to a problem occurring on SSR with the ExportAsService dependency, which use window object, the service can't be injected.
7483
So we need to instantiate the class directly based on current the platform */
7584
if (isPlatformBrowser(this.platformId)) {
7685
import('../../../../core/export-service/browser-export.service').then((s) => {
77-
this.exportService = new s.BrowserExportService(this.platformId);
86+
this.exportService = new s.BrowserExportService(this.platformId, this._window, this.document);
7887
});
7988
} else {
8089
import('../../../../core/export-service/server-export.service').then((s) => {

‎src/app/statistics-page/cris-statistics-page/statistics-chart/statistics-chart-line/statistics-chart-line.component.spec.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { DOCUMENT } from '@angular/common';
12
import {
23
ChangeDetectionStrategy,
34
DebugElement,
@@ -12,6 +13,10 @@ import { By } from '@angular/platform-browser';
1213
import { NoopAnimationsModule } from '@angular/platform-browser/animations';
1314
import { TranslateModule } from '@ngx-translate/core';
1415
import { of as observableOf } from 'rxjs';
16+
import {
17+
NativeWindowRef,
18+
NativeWindowService,
19+
} from 'src/app/core/services/window.service';
1520

1621
import { BrowserExportService } from '../../../../core/export-service/browser-export.service';
1722
import { REPORT_DATA } from '../../../../core/statistics/data-report.service';
@@ -197,6 +202,8 @@ describe('StatisticsChartLineComponent', () => {
197202
{ provide: REPORT_DATA, useValue: selectedReport },
198203
{ provide: BrowserExportService, useValue: ExportServiceStub },
199204
{ provide: 'categoryType', useValue: 'mainReports' },
205+
{ provide: NativeWindowService, useValue: new NativeWindowRef() },
206+
{ provide: DOCUMENT, useValue: document },
200207
],
201208
schemas: [NO_ERRORS_SCHEMA],
202209
}).overrideComponent(StatisticsChartLineComponent, {

0 commit comments

Comments
 (0)