Skip to content

Commit

Permalink
fix(UVE): Ensure Window Object Re-Initialization in UVE iframe (#30989)
Browse files Browse the repository at this point in the history
This pull request includes several changes aimed at improving the
functionality and testing of the `EditEmaEditorComponent` in the
`core-web` library. The key updates involve refactoring the iframe URL
handling, enhancing the test coverage, and improving the inline editing
feature.

### Refactoring and Improvements:
* Updated the iframe `src` binding to use `uveStore.$iframeURL()`
instead of `$editorProps().iframe.src` in
`edit-ema-editor.component.html`.
* Refactored the `setIframeContent` method to include an
`enableInlineEdit` parameter and moved the inline editing script
handling to a new method `handleInlineScripts` in
`edit-ema-editor.component.ts`.
[[1]](diffhunk://#diff-24dc496db1eb6feb3e10a031baa4b4b63c20f1cd02ade574065f6b967f11c365L195-L215)
[[2]](diffhunk://#diff-24dc496db1eb6feb3e10a031baa4b4b63c20f1cd02ade574065f6b967f11c365L685-R718)

### Testing Enhancements:
* Added event listener and dispatcher methods to the iframe mock in
`edit-ema-editor.component.spec.ts` to simulate iframe load events.
* Modified multiple test cases to dispatch iframe load events and verify
the iframe content in `edit-ema-editor.component.spec.ts`.
[[1]](diffhunk://#diff-34ddc5fbacaf04b962f2037385ed284310d5faf35ba409d5705b2caadd5d796aL2617-R2625)
[[2]](diffhunk://#diff-34ddc5fbacaf04b962f2037385ed284310d5faf35ba409d5705b2caadd5d796aR2650-R2652)
[[3]](diffhunk://#diff-34ddc5fbacaf04b962f2037385ed284310d5faf35ba409d5705b2caadd5d796aL2668-R2686)
[[4]](diffhunk://#diff-34ddc5fbacaf04b962f2037385ed284310d5faf35ba409d5705b2caadd5d796aR2826-R2842)
[[5]](diffhunk://#diff-34ddc5fbacaf04b962f2037385ed284310d5faf35ba409d5705b2caadd5d796aR2858-R2860)

### Store and State Management:
* Introduced a new computed property `$iframeURL` in the `withEditor`
store to dynamically generate iframe URLs, including handling
traditional pages by returning `about:blank`.
[[1]](diffhunk://#diff-86e692578757ed7f4f6cba5d0aeb07641312f3b17885825d1a45987153ae87f0L39-R61)
[[2]](diffhunk://#diff-86e692578757ed7f4f6cba5d0aeb07641312f3b17885825d1a45987153ae87f0L124-L127)
[[3]](diffhunk://#diff-86e692578757ed7f4f6cba5d0aeb07641312f3b17885825d1a45987153ae87f0L140-L141)
[[4]](diffhunk://#diff-86e692578757ed7f4f6cba5d0aeb07641312f3b17885825d1a45987153ae87f0L150)
[[5]](diffhunk://#diff-86e692578757ed7f4f6cba5d0aeb07641312f3b17885825d1a45987153ae87f0R193-R203)
* Removed the `src` property from the `EditorProps` interface as it is
now handled by the computed `$iframeURL`.

### Code Cleanup:
* Removed unnecessary `requestAnimationFrame` calls and related code
from `edit-ema-editor.component.ts`.
[[1]](diffhunk://#diff-24dc496db1eb6feb3e10a031baa4b4b63c20f1cd02ade574065f6b967f11c365L195-L215)
[[2]](diffhunk://#diff-24dc496db1eb6feb3e10a031baa4b4b63c20f1cd02ade574065f6b967f11c365L685-R718)
* Cleaned up unused imports and reorganized utility function usage in
`withEditor.ts`.

These changes collectively enhance the maintainability, functionality,
and testability of the `EditEmaEditorComponent`.

### Video


https://github.com/user-attachments/assets/2d020e92-0dd3-4b80-8bb4-028c5a8b252a
  • Loading branch information
rjvelazco authored Dec 20, 2024
1 parent a2c70ab commit a5315a4
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 69 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
class="iframe-wrapper">
<iframe
(load)="onIframePageLoad()"
[src]="$editorProps().iframe.src | safeUrl"
[src]="uveStore.$iframeURL() | safeUrl"
[title]="host"
[ngStyle]="{
pointerEvents: $editorProps().iframe.pointerEvents,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,14 @@ const messagesMock = {

const IFRAME_MOCK = {
nativeElement: {
addEventListener: function (_event, cb) {
this.registerListener.push(cb);
},
dispatchEvent: function (event) {
this.registerListener.forEach((cb) => cb(event));
},
registerListener: [],
contentWindow: document.defaultView,
contentDocument: {
getElementsByTagName: () => [],
querySelectorAll: () => [],
Expand Down Expand Up @@ -2614,7 +2622,7 @@ describe('EditEmaEditorComponent', () => {
const iframe = spectator.debugElement.query(By.css('[data-testId="iframe"]'));

expect(iframe.nativeElement.src).toBe(
'http://localhost:3000/index?clientHost=http%3A%2F%2Flocalhost%3A3000&language_id=1&com.dotmarketing.persona.id=modes.persona.no.persona&variantName=DEFAULT'
'http://localhost:3000/page-one?clientHost=http%3A%2F%2Flocalhost%3A3000&language_id=1&com.dotmarketing.persona.id=modes.persona.no.persona&variantName=DEFAULT'
);
});

Expand All @@ -2639,6 +2647,9 @@ describe('EditEmaEditorComponent', () => {
By.css('[data-testId="iframe"]')
);

iframe.nativeElement.dispatchEvent(new Event('load'));
spectator.detectChanges();

expect(iframe.nativeElement.contentDocument.body.innerHTML).toContain(
'<div>hello world</div>'
);
Expand All @@ -2665,11 +2676,14 @@ describe('EditEmaEditorComponent', () => {
language_id: '4',
'com.dotmarketing.persona.id': DEFAULT_PERSONA.identifier
});
spectator.detectChanges();

spectator.detectChanges();
jest.runOnlyPendingTimers();

expect(iframe.nativeElement.src).toBe('http://localhost/'); //When dont have src, the src is the same as the current page
iframe.nativeElement.dispatchEvent(new Event('load'));
spectator.detectChanges();

expect(iframe.nativeElement.src).toContain('about:blank'); //When dont have src, the src is the same as the current page
expect(iframe.nativeElement.contentDocument.body.innerHTML).toContain(
'<div>New Content - Hello World</div>'
);
Expand Down Expand Up @@ -2809,24 +2823,23 @@ describe('EditEmaEditorComponent', () => {

describe('script and styles injection', () => {
let iframeDocument: Document;
let iframeElement: HTMLIFrameElement;
let spy: jest.SpyInstance;

beforeEach(() => {
jest.spyOn(window, 'requestAnimationFrame').mockImplementation((cb) => {
cb(0); // Pass a dummy value to satisfy the expected argument count

return 0;
});

// eslint-disable-next-line @typescript-eslint/no-explicit-any
spectator.component.iframe = IFRAME_MOCK as any;
iframeDocument = spectator.component.iframe.nativeElement.contentDocument;
iframeElement = spectator.component.iframe.nativeElement;
iframeDocument = iframeElement.contentDocument;
spy = jest.spyOn(iframeDocument, 'write');
});

it('should add script and styles to iframe', () => {
spectator.component.setIframeContent(`<head></head></body></body>`);

iframeElement.dispatchEvent(new Event('load'));
spectator.detectChanges();

expect(spy).toHaveBeenCalled();
expect(iframeDocument.body.innerHTML).toContain(
`<script src="${SDK_EDITOR_SCRIPT_SOURCE}"></script>`
Expand All @@ -2842,6 +2855,9 @@ describe('EditEmaEditorComponent', () => {
it('should add script and styles to iframe for advance templates', () => {
spectator.component.setIframeContent(`<div>Advanced Template</div>`);

iframeElement.dispatchEvent(new Event('load'));
spectator.detectChanges();

expect(spy).toHaveBeenCalled();
expect(iframeDocument.body.innerHTML).toContain(
`<script src="${SDK_EDITOR_SCRIPT_SOURCE}"></script>`
Expand All @@ -2854,10 +2870,6 @@ describe('EditEmaEditorComponent', () => {
'[data-dot-object="contentlet"].empty-contentlet'
);
});

afterEach(() => {
(window.requestAnimationFrame as jest.Mock).mockRestore();
});
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,27 +192,14 @@ export class EditEmaEditorComponent implements OnInit, OnDestroy {
return;
}

this.setIframeContent(code);

requestAnimationFrame(() => {
/**
* The status of isClientReady is changed outside of editor
* so we need to set it to true here to avoid the editor to be in a loading state
* This is only for traditional pages. For Headless, the isClientReady is set from the client application
*/
this.uveStore.setIsClientReady(true);
const win = this.contentWindow;
if (enableInlineEdit) {
this.inlineEditingService.injectInlineEdit(this.iframe);
} else {
this.inlineEditingService.removeInlineEdit(this.iframe);
}
this.setIframeContent(code, enableInlineEdit);

fromEvent(win, 'click').subscribe((e: MouseEvent) => {
this.handleInternalNav(e);
this.handleInlineEditing(e); // If inline editing is not active this will do nothing
});
});
/**
* The status of isClientReady is changed outside of editor
* so we need to set it to true here to avoid the editor to be in a loading state
* This is only for traditional pages. For Headless, the isClientReady is set from the client application
*/
this.uveStore.setIsClientReady(true);

return;
},
Expand Down Expand Up @@ -682,23 +669,53 @@ export class EditEmaEditorComponent implements OnInit, OnDestroy {
* @param code - The code to be added to the iframe.
* @memberof EditEmaEditorComponent
*/
setIframeContent(code: string) {
requestAnimationFrame(() => {
const doc = this.iframe?.nativeElement.contentDocument;
setIframeContent(code: string, enableInlineEdit = false): void {
const iframeElement = this.iframe?.nativeElement;

if (doc) {
const newFile = this.inyectCodeToVTL(code);
if (!iframeElement) {
return;
}

doc.open();
doc.write(newFile);
doc.close();
iframeElement.addEventListener('load', () => {
const doc = iframeElement.contentDocument;
const newDoc = this.inyectCodeToVTL(code);

this.uveStore.setOgTags(this.dotSeoMetaTagsUtilService.getMetaTags(doc));
this.ogTagsResults$ = this.dotSeoMetaTagsService
.getMetaTagsResults(doc)
.pipe(take(1));
if (!doc) {
return;
}

doc.open();
doc.write(newDoc);
doc.close();

this.uveStore.setOgTags(this.dotSeoMetaTagsUtilService.getMetaTags(doc));
this.ogTagsResults$ = this.dotSeoMetaTagsService.getMetaTagsResults(doc).pipe(take(1));
this.handleInlineScripts(enableInlineEdit);
});
}

/**
* Handle the Injection and removal of the inline editing scripts
*
* @param {boolean} enableInlineEdit
* @return {*}
* @memberof EditEmaEditorComponent
*/
handleInlineScripts(enableInlineEdit: boolean) {
const win = this.contentWindow;

fromEvent(win, 'click').subscribe((e: MouseEvent) => {
this.handleInternalNav(e);
this.handleInlineEditing(e); // If inline editing is not active this will do nothing
});

if (enableInlineEdit) {
this.inlineEditingService.injectInlineEdit(this.iframe);

return;
}

this.inlineEditingService.removeInlineEdit(this.iframe);
}

protected handleNgEvent({ event, actionPayload, clientAction }: DialogAction) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ export interface EditorProps {
width: string;
height: string;
};
src: string;
pointerEvents: string;
opacity: string;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,23 @@ describe('withEditor', () => {
});
});

describe('$iframeURL', () => {
it("should return the iframe's URL", () => {
expect(store.$iframeURL()).toBe(
'http://localhost:3000/test-url?language_id=1&com.dotmarketing.persona.id=dot%3Apersona&variantName=DEFAULT&clientHost=http%3A%2F%2Flocalhost%3A3000'
);
});

it('should contain `about:blanck` in src when the page is traditional', () => {
patchState(store, {
pageAPIResponse: MOCK_RESPONSE_VTL,
isTraditionalPage: true
});

expect(store.$iframeURL()).toContain('about:blank');
});
});

describe('$editorProps', () => {
it('should return the expected data on init', () => {
expect(store.$editorProps()).toEqual({
Expand All @@ -651,7 +668,6 @@ describe('withEditor', () => {
iframe: {
opacity: '0.5',
pointerEvents: 'auto',
src: 'http://localhost:3000/test-url?language_id=1&com.dotmarketing.persona.id=dot%3Apersona&variantName=DEFAULT&clientHost=http%3A%2F%2Flocalhost%3A3000',
wrapper: null
},
progressBar: true,
Expand Down Expand Up @@ -720,15 +736,6 @@ describe('withEditor', () => {
expect(store.$editorProps().iframe.pointerEvents).toBe('none');
});

it('should have src as empty when the page is traditional', () => {
patchState(store, {
pageAPIResponse: MOCK_RESPONSE_VTL,
isTraditionalPage: true
});

expect(store.$editorProps().iframe.src).toBe('');
});

it('should have a wrapper when a device is present', () => {
const device = mockDotDevices[0] as DotDeviceWithIcon;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,29 @@ import {
PositionPayload
} from '../../../shared/models';
import {
sanitizeURL,
createPageApiUrlWithQueryParams,
mapContainerStructureToArrayOfContainers,
getPersonalization,
areContainersEquals,
getEditorStates
getEditorStates,
createPageApiUrlWithQueryParams,
sanitizeURL
} from '../../../utils';
import { UVEState } from '../../models';
import { withClient } from '../client/withClient';

const buildIframeURL = ({ pageURI, params, isTraditionalPage }) => {
if (isTraditionalPage) {
// Force iframe reload on every page load to avoid caching issues and window dirty state
return `about:blank?t=${Date.now()}`;
}

const pageAPIQueryParams = createPageApiUrlWithQueryParams(pageURI, params);
const origin = params.clientHost || window.location.origin;
const url = new URL(pageAPIQueryParams, origin);

return sanitizeURL(url.toString());
};

const initialState: EditorState = {
bounds: [],
state: EDITOR_STATE.IDLE,
Expand Down Expand Up @@ -122,10 +135,6 @@ export function withEditor() {

const { dragIsActive, isScrolling } = getEditorStates(state);

const url = sanitizeURL(params?.url);

const pageAPIQueryParams = createPageApiUrlWithQueryParams(url, params);

const showDialogs = canEditPage && isEditState;
const showBlockEditorSidebar = canEditPage && isEditState && isEnterprise;

Expand All @@ -142,8 +151,6 @@ export function withEditor() {
const shouldShowSeoResults = socialMedia && ogTags;

const iframeOpacity = isLoading || !isPageReady ? '0.5' : '1';
const origin = params.clientHost || window.location.origin;
const iframeURL = new URL(pageAPIQueryParams, origin);

return {
showDialogs,
Expand All @@ -152,7 +159,6 @@ export function withEditor() {
iframe: {
opacity: iframeOpacity,
pointerEvents: dragIsActive ? 'none' : 'auto',
src: !isTraditionalPage ? iframeURL.href : '',
wrapper: device
? {
width: `${device.cssWidth}${BASE_IFRAME_MEASURE_UNIT}`,
Expand Down Expand Up @@ -189,6 +195,16 @@ export function withEditor() {
}
: null
};
}),
$iframeURL: computed<string>(() => {
const page = store.pageAPIResponse().page;
const url = buildIframeURL({
pageURI: page?.pageURI,
params: store.pageParams(),
isTraditionalPage: untracked(() => store.isTraditionalPage())
});

return url;
})
};
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ export function withLoad() {
pageAsset?.page,
currentUser
);

const isTraditionalPage = !pageParams.clientHost;

patchState(store, {
Expand Down

0 comments on commit a5315a4

Please sign in to comment.