From 37c4f483072def9e9e60097f3fedfac8f9cd4ca7 Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Mon, 29 Jun 2026 11:00:43 -0400 Subject: [PATCH 01/18] feat(image-editor): wire Save temp-file flow and focal-point persistence Add a withSave feature (save events + saveEditedImage GET) that builds the contentAsset filter URL with _imageToolSaveFile=true, closes the dialog with the returned temp file on success and surfaces the error on failure. Fold the focal point into the save chain (epsilon-compared against the seeded baseline), seed it on open from ImageEditorOpenParams, widen isDirty to detect a focal move, and add a footer Save button that shows a loading spinner while busy or saving. --- .../dot-image-editor-footer.component.html | 7 + .../dot-image-editor-footer.component.spec.ts | 34 ++++- .../dot-image-editor-footer.component.ts | 13 +- .../dot-image-editor.component.html | 6 + .../dot-image-editor.component.spec.ts | 56 +++++++- .../dot-image-editor.component.ts | 15 +- .../src/lib/models/image-editor.models.ts | 26 +++- .../lib/services/dot-image-editor.service.ts | 18 ++- .../src/lib/store/features/index.ts | 1 + .../store/features/with-asset.feature.spec.ts | 66 +++++++++ .../lib/store/features/with-asset.feature.ts | 6 +- .../features/with-preview.feature.spec.ts | 75 ++++++++++ .../store/features/with-preview.feature.ts | 17 ++- .../store/features/with-save.feature.spec.ts | 134 ++++++++++++++++++ .../lib/store/features/with-save.feature.ts | 77 ++++++++++ .../src/lib/store/image-editor.events.ts | 7 +- .../src/lib/store/image-editor.state.ts | 3 + .../src/lib/store/image-editor.store.ts | 10 +- .../utils/image-filter-url.builder.spec.ts | 83 ++++++++++- .../src/lib/utils/image-filter-url.builder.ts | 75 +++++++++- .../WEB-INF/messages/Language.properties | 2 + 21 files changed, 704 insertions(+), 27 deletions(-) create mode 100644 core-web/libs/image-editor/src/lib/store/features/with-asset.feature.spec.ts create mode 100644 core-web/libs/image-editor/src/lib/store/features/with-preview.feature.spec.ts create mode 100644 core-web/libs/image-editor/src/lib/store/features/with-save.feature.spec.ts create mode 100644 core-web/libs/image-editor/src/lib/store/features/with-save.feature.ts diff --git a/core-web/libs/image-editor/src/lib/components/dot-image-editor-footer/dot-image-editor-footer.component.html b/core-web/libs/image-editor/src/lib/components/dot-image-editor-footer/dot-image-editor-footer.component.html index 96fef02f7ea..dc84678f4cd 100644 --- a/core-web/libs/image-editor/src/lib/components/dot-image-editor-footer/dot-image-editor-footer.component.html +++ b/core-web/libs/image-editor/src/lib/components/dot-image-editor-footer/dot-image-editor-footer.component.html @@ -16,4 +16,11 @@ download + + diff --git a/core-web/libs/image-editor/src/lib/components/dot-image-editor-footer/dot-image-editor-footer.component.spec.ts b/core-web/libs/image-editor/src/lib/components/dot-image-editor-footer/dot-image-editor-footer.component.spec.ts index 854d439579a..afcbb9ba2fc 100644 --- a/core-web/libs/image-editor/src/lib/components/dot-image-editor-footer/dot-image-editor-footer.component.spec.ts +++ b/core-web/libs/image-editor/src/lib/components/dot-image-editor-footer/dot-image-editor-footer.component.spec.ts @@ -27,25 +27,28 @@ describe('DotImageEditorFooterComponent', () => { let dispatcher: SpyObject; const isBusy = signal(false); + const saveStatus = signal<'idle' | 'saving' | 'error'>('idle'); const createComponent = createComponentFactory({ component: DotImageEditorFooterComponent, imports: [DotMessagePipe], providers: [mockProvider(DotMessageService, { get: jest.fn((key: string) => key) })], - componentProviders: [Dispatcher, mockProvider(ImageEditorStore, { isBusy })] + componentProviders: [Dispatcher, mockProvider(ImageEditorStore, { isBusy, saveStatus })] }); beforeEach(() => { isBusy.set(false); + saveStatus.set('idle'); spectator = createComponent(); dispatcher = spectator.inject(Dispatcher, true); jest.spyOn(dispatcher, 'dispatch'); }); - it('should render the cancel and download actions', () => { + it('should render the cancel, download and save actions', () => { expect(spectator.query(byTestId('image-editor-cancel-btn'))).toBeTruthy(); expect(spectator.query(byTestId('image-editor-download-btn'))).toBeTruthy(); + expect(spectator.query(byTestId('image-editor-save-btn'))).toBeTruthy(); }); it('should emit cancel when Cancel is clicked', () => { @@ -71,4 +74,31 @@ describe('DotImageEditorFooterComponent', () => { expect(nativeButton(spectator, 'image-editor-download-btn').disabled).toBe(true); }); + + it('should dispatch saveRequested when Save is clicked', () => { + spectator.click(nativeButton(spectator, 'image-editor-save-btn')); + + expect(dispatcher.dispatch).toHaveBeenCalledWith( + imageEditorLifecycleEvents.saveRequested(), + { scope: 'self' } + ); + }); + + it('should show a loading spinner and disable Save while a save is in flight', () => { + saveStatus.set('saving'); + spectator.detectChanges(); + + const button = nativeButton(spectator, 'image-editor-save-btn'); + expect(button.disabled).toBe(true); + expect(button.classList).toContain('p-button-loading'); + }); + + it('should show a loading spinner and disable Save while a filter is applying (busy)', () => { + isBusy.set(true); + spectator.detectChanges(); + + const button = nativeButton(spectator, 'image-editor-save-btn'); + expect(button.disabled).toBe(true); + expect(button.classList).toContain('p-button-loading'); + }); }); diff --git a/core-web/libs/image-editor/src/lib/components/dot-image-editor-footer/dot-image-editor-footer.component.ts b/core-web/libs/image-editor/src/lib/components/dot-image-editor-footer/dot-image-editor-footer.component.ts index d7a3b7fe1c7..e8e31ada3fc 100644 --- a/core-web/libs/image-editor/src/lib/components/dot-image-editor-footer/dot-image-editor-footer.component.ts +++ b/core-web/libs/image-editor/src/lib/components/dot-image-editor-footer/dot-image-editor-footer.component.ts @@ -11,9 +11,9 @@ import { ImageEditorStore } from '../../store/image-editor.store'; /** * Footer action bar of the image editor dialog. Reads readiness from the - * {@link ImageEditorStore} (`isBusy`) and dispatches the download lifecycle event. - * Cancel is surfaced as an output so the owning dialog component controls closing. - * (Saving the edited image back to the field is handled in a separate issue.) + * {@link ImageEditorStore} (`isBusy`/`saveStatus`) and dispatches the download and + * save lifecycle events. Cancel is surfaced as an output so the owning dialog + * component controls closing. */ @Component({ selector: 'dot-image-editor-footer', @@ -25,7 +25,7 @@ export class DotImageEditorFooterComponent { /** Image editor state store, provided by the owning dialog component. */ protected readonly store = inject(ImageEditorStore); - /** Lifecycle event dispatcher for the download action. */ + /** Lifecycle event dispatcher for the download and save actions. */ protected readonly dispatch = injectDispatch(imageEditorLifecycleEvents); /** Emitted when the user clicks Cancel; the dialog owner closes the editor. */ @@ -35,4 +35,9 @@ export class DotImageEditorFooterComponent { protected onDownload(): void { this.dispatch.downloadRequested(); } + + /** Dispatches a save of the current edits into a temp file. */ + protected onSave(): void { + this.dispatch.saveRequested(); + } } diff --git a/core-web/libs/image-editor/src/lib/components/dot-image-editor/dot-image-editor.component.html b/core-web/libs/image-editor/src/lib/components/dot-image-editor/dot-image-editor.component.html index 911f69efdf5..7fa7901c4df 100644 --- a/core-web/libs/image-editor/src/lib/components/dot-image-editor/dot-image-editor.component.html +++ b/core-web/libs/image-editor/src/lib/components/dot-image-editor/dot-image-editor.component.html @@ -10,6 +10,12 @@
+ @if (store.saveStatus() === 'error') { + + } +
diff --git a/core-web/libs/image-editor/src/lib/components/dot-image-editor/dot-image-editor.component.spec.ts b/core-web/libs/image-editor/src/lib/components/dot-image-editor/dot-image-editor.component.spec.ts index 351a0e5d372..5847ae23bde 100644 --- a/core-web/libs/image-editor/src/lib/components/dot-image-editor/dot-image-editor.component.spec.ts +++ b/core-web/libs/image-editor/src/lib/components/dot-image-editor/dot-image-editor.component.spec.ts @@ -15,6 +15,7 @@ import { Confirmation, ConfirmationService, ConfirmEventType } from 'primeng/api import { DynamicDialogConfig, DynamicDialogRef } from 'primeng/dynamicdialog'; import { DotMessageService } from '@dotcms/data-access'; +import { DotCMSTempFile } from '@dotcms/dotcms-models'; import { DotImageEditorComponent } from './dot-image-editor.component'; @@ -41,6 +42,8 @@ function describeWith(label: string, data: ImageEditorOpenParams): void { const canUndo = signal(false); const canRedo = signal(false); const isFullscreen = signal(false); + const saveStatus = signal<'idle' | 'saving' | 'error'>('idle'); + const saveError = signal(null); const createComponent = createComponentFactory({ component: DotImageEditorComponent, @@ -57,7 +60,14 @@ function describeWith(label: string, data: ImageEditorOpenParams): void { // mocking only the store. componentProviders: [ ConfirmationService, - mockProvider(ImageEditorStore, { isDirty, canUndo, canRedo, isFullscreen }) + mockProvider(ImageEditorStore, { + isDirty, + canUndo, + canRedo, + isFullscreen, + saveStatus, + saveError + }) ], // Isolate the shell from the children's own store/dispatch wiring. overrideComponents: [ @@ -94,6 +104,8 @@ function describeWith(label: string, data: ImageEditorOpenParams): void { canUndo.set(false); canRedo.set(false); isFullscreen.set(false); + saveStatus.set('idle'); + saveError.set(null); // Spy before creation so the constructor's assetRequested dispatch is // captured (injectDispatch dispatches through Dispatcher.prototype). @@ -215,6 +227,48 @@ function describeWith(label: string, data: ImageEditorOpenParams): void { expect(confirmationService.confirm).toHaveBeenCalledTimes(1); }); + describe('save lifecycle', () => { + const tempFile: DotCMSTempFile = { + fileName: 'edited.png', + folder: 'shared', + id: 'temp_578026b7cc', + image: true, + length: 1024, + mimeType: 'image/png', + referenceUrl: '/dA/temp_578026b7cc', + thumbnailUrl: '/dA/temp_578026b7cc/thumb' + }; + + it('should close with the temp file when a save succeeds', () => { + dispatcher.dispatch(imageEditorLifecycleEvents.saveSucceeded(tempFile)); + + expect(dialogRef.close).toHaveBeenCalledWith(tempFile); + }); + + it('should NOT close when a save fails', () => { + dispatcher.dispatch(imageEditorLifecycleEvents.saveFailed(new Error('boom'))); + + expect(dialogRef.close).not.toHaveBeenCalled(); + }); + + it('should surface the save error when saveStatus is error', () => { + saveStatus.set('error'); + saveError.set('Failed to save image'); + spectator.detectChanges(); + + const error = spectator.query(byTestId('image-editor-save-error')); + expect(error).toExist(); + expect(error?.textContent?.trim()).toBe('Failed to save image'); + }); + + it('should not render the save error when saveStatus is idle', () => { + saveStatus.set('idle'); + spectator.detectChanges(); + + expect(spectator.query(byTestId('image-editor-save-error'))).not.toExist(); + }); + }); + describe('undo/redo shortcuts', () => { it('should dispatch undoRequested on Ctrl/Cmd+Z when undo is available', () => { canUndo.set(true); diff --git a/core-web/libs/image-editor/src/lib/components/dot-image-editor/dot-image-editor.component.ts b/core-web/libs/image-editor/src/lib/components/dot-image-editor/dot-image-editor.component.ts index adfca0f5c9f..9afe4c5806a 100644 --- a/core-web/libs/image-editor/src/lib/components/dot-image-editor/dot-image-editor.component.ts +++ b/core-web/libs/image-editor/src/lib/components/dot-image-editor/dot-image-editor.component.ts @@ -1,7 +1,8 @@ -import { injectDispatch } from '@ngrx/signals/events'; +import { Events, injectDispatch } from '@ngrx/signals/events'; import { DOCUMENT } from '@angular/common'; import { ChangeDetectionStrategy, Component, effect, inject } from '@angular/core'; +import { takeUntilDestroyed } from '@angular/core/rxjs-interop'; import { ConfirmationService, ConfirmEventType } from 'primeng/api'; import { ConfirmDialogModule } from 'primeng/confirmdialog'; @@ -27,8 +28,8 @@ import { DotImageEditorPanelsComponent } from '../dot-image-editor-panels/dot-im * Full-screen "Edit image" modal shell, opened through PrimeNG's `DialogService`. * Assembles the header, canvas, side panels and footer over a single * {@link ImageEditorStore} instance scoped to this dialog. It owns the dialog - * lifecycle: it requests the asset on init and guards close/cancel against unsaved - * edits (saving the edited image is deferred to a separate issue). + * lifecycle: it requests the asset on init, closes with the saved temp file when a + * save succeeds, and guards close/cancel against unsaved edits. */ @Component({ selector: 'dot-image-editor', @@ -66,6 +67,7 @@ export class DotImageEditorComponent { readonly #dotMessageService = inject(DotMessageService); readonly #dispatch = injectDispatch(imageEditorLifecycleEvents); readonly #historyDispatch = injectDispatch(imageEditorHistoryEvents); + readonly #events = inject(Events); readonly #document = inject(DOCUMENT); // The PrimeNG dialog hosting this editor: injectable because the dialog content // is declared inside `` in DynamicDialog's template, so we sit in the @@ -84,6 +86,13 @@ export class DotImageEditorComponent { // The editor owns its dialog, so it owns full-screen too: resize the host // `.p-dialog` to fill the viewport whenever `isFullscreen` flips. effect(() => this.#applyFullscreen(this.store.isFullscreen())); + + // A successful save resolves the editor with the staged temp file; a failed + // save keeps the dialog open and surfaces the error in the template. + this.#events + .on(imageEditorLifecycleEvents.saveSucceeded) + .pipe(takeUntilDestroyed()) + .subscribe(({ payload }) => this.#dialogRef.close(payload)); } /** diff --git a/core-web/libs/image-editor/src/lib/models/image-editor.models.ts b/core-web/libs/image-editor/src/lib/models/image-editor.models.ts index b4d8f1fd9e6..058fd273b0b 100644 --- a/core-web/libs/image-editor/src/lib/models/image-editor.models.ts +++ b/core-web/libs/image-editor/src/lib/models/image-editor.models.ts @@ -30,6 +30,9 @@ export interface ImageRect { /** Loading lifecycle of the preview image. */ export type PreviewStatus = 'idle' | 'loading' | 'loaded' | 'error'; +/** Lifecycle of the Save (temp-file) request. */ +export type SaveStatus = 'idle' | 'saving' | 'error'; + /** Logical category an applied edit belongs to, used for grouping and labels. */ export type FilterCategory = | 'adjust' @@ -51,6 +54,9 @@ export type FilterName = | 'Jpeg' | 'WebP' | 'Quality' + // Focal point is injected into the chain only when saving (never in the preview + // URL); the registry key resolves case-insensitively to `focalpoint`. + | 'FocalPoint' // libvips-only modern format (AV1); registered lowercase as `avif`. | 'avif'; @@ -187,6 +193,11 @@ export interface ImageEditorOpenParams { byInode?: boolean; fileName?: string; mimeType?: string; + /** + * Normalized 0..1 focal point already stored on the asset, used to seed the + * editor so an existing focal point is preserved on Save unless the user moves it. + */ + focalPoint?: NormalizedPoint; } /** @@ -226,17 +237,26 @@ export interface ImageEditorState { zoom: ZoomState; /** * Normalized 0..1 focal point held as editor state only. It is NOT a filter slice - * (it never enters the preview filter chain nor the edit history) and is NOT - * persisted on its own — it would be committed alongside the other edits during - * the (separate) Save flow. See {@link withFocalPoint}. + * (it never enters the preview filter chain nor the edit history); it is folded + * into the chain only when saving and committed alongside the other edits. See + * {@link withFocalPoint} and {@link withSave}. */ focalPoint: NormalizedPoint; + /** + * The focal point the asset was opened with, used as the dirty/save baseline so a + * move-and-back is detected as pristine. + */ + seededFocalPoint: NormalizedPoint; /** Tool currently selected on the canvas. */ activeTool: ActiveTool; /** Loading lifecycle of the preview image. */ previewStatus: PreviewStatus; /** Consecutive silent retries of the current failing preview (reset on success). */ previewRetries: number; + /** Lifecycle of the Save (temp-file) request. */ + saveStatus: SaveStatus; + /** Last Save error message surfaced to the user, or `null`. */ + saveError: string | null; /** Last error message surfaced to the user, or `null`. */ error: string | null; /** Ordered undo/redo history of applied edits. */ diff --git a/core-web/libs/image-editor/src/lib/services/dot-image-editor.service.ts b/core-web/libs/image-editor/src/lib/services/dot-image-editor.service.ts index 0ab8dec5fcf..9cc5bb5e56a 100644 --- a/core-web/libs/image-editor/src/lib/services/dot-image-editor.service.ts +++ b/core-web/libs/image-editor/src/lib/services/dot-image-editor.service.ts @@ -6,6 +6,8 @@ import { Injectable, inject } from '@angular/core'; import { catchError, map } from 'rxjs/operators'; +import { DotCMSTempFile } from '@dotcms/dotcms-models'; + import { AssetMeta, ImageEditorAssetContext, @@ -14,9 +16,9 @@ import { /** * Data-access service for the image editor: resolves asset metadata, queries the - * verified preview blob and file sizes, and triggers client-side downloads. All - * read-only/metadata calls are non-fatal and never throw. (Saving the edited image - * is handled in a separate issue.) + * verified preview blob and file sizes, triggers client-side downloads, and saves + * the edited image into a temp file. All read-only/metadata calls are non-fatal and + * never throw. */ @Injectable({ providedIn: 'root' }) export class DotImageEditorService { @@ -127,6 +129,16 @@ export class DotImageEditorService { anchor.remove(); } + /** + * Saves the edited image by GET-ting the filter/save URL, which makes the + * servlet stage the filtered render into a temp file and return its descriptor. + * @param url - The Save URL built by `buildSaveUrl` + * @returns The staged temp file descriptor + */ + saveEditedImage(url: string): Observable { + return this.#http.get(url); + } + #resolveNaturalDimensions(url: string): Observable { return new Observable((subscriber) => { const image = this.#document.createElement('img'); diff --git a/core-web/libs/image-editor/src/lib/store/features/index.ts b/core-web/libs/image-editor/src/lib/store/features/index.ts index f323309868b..befc99ecec6 100644 --- a/core-web/libs/image-editor/src/lib/store/features/index.ts +++ b/core-web/libs/image-editor/src/lib/store/features/index.ts @@ -6,5 +6,6 @@ export { withFileInfo } from './with-file-info.feature'; export { withFocalPoint } from './with-focal-point.feature'; export { withHistory } from './with-history.feature'; export { withPreview } from './with-preview.feature'; +export { withSave } from './with-save.feature'; export { withTransform } from './with-transform.feature'; export { withView } from './with-view.feature'; diff --git a/core-web/libs/image-editor/src/lib/store/features/with-asset.feature.spec.ts b/core-web/libs/image-editor/src/lib/store/features/with-asset.feature.spec.ts new file mode 100644 index 00000000000..0d5432a6eca --- /dev/null +++ b/core-web/libs/image-editor/src/lib/store/features/with-asset.feature.spec.ts @@ -0,0 +1,66 @@ +import { signalStore, withState } from '@ngrx/signals'; +import { Dispatcher, injectDispatch } from '@ngrx/signals/events'; +import { of } from 'rxjs'; + +import { Injector, runInInjectionContext } from '@angular/core'; +import { TestBed } from '@angular/core/testing'; + +import { withAsset } from './with-asset.feature'; + +import { ImageEditorOpenParams } from '../../models/image-editor.models'; +import { DotImageEditorService } from '../../services/dot-image-editor.service'; +import { imageEditorLifecycleEvents } from '../image-editor.events'; +import { initialFocalPointState, initialImageEditorState } from '../image-editor.state'; + +const AssetStore = signalStore(withState(initialImageEditorState), withAsset()); + +const BASE_PARAMS: ImageEditorOpenParams = { + inode: 'inode-1', + variable: 'fileAsset', + fieldName: 'fileAsset', + fileName: 'photo.png', + mimeType: 'image/png' +}; + +describe('withAsset', () => { + let store: InstanceType; + let lifecycle: ReturnType>; + + beforeEach(() => { + TestBed.configureTestingModule({ + providers: [ + AssetStore, + Dispatcher, + { + provide: DotImageEditorService, + useValue: { + loadAssetMeta: jest + .fn() + .mockReturnValue( + of({ naturalWidth: 800, naturalHeight: 600, originalBytes: 5000 }) + ) + } + } + ] + }); + const injector = TestBed.inject(Injector); + store = TestBed.inject(AssetStore); + runInInjectionContext(injector, () => { + lifecycle = injectDispatch(imageEditorLifecycleEvents); + }); + }); + + it('seeds focalPoint and seededFocalPoint from the open params', () => { + lifecycle.assetRequested({ ...BASE_PARAMS, focalPoint: { x: 0.3, y: 0.7 } }); + + expect(store.focalPoint()).toEqual({ x: 0.3, y: 0.7 }); + expect(store.seededFocalPoint()).toEqual({ x: 0.3, y: 0.7 }); + }); + + it('defaults focalPoint and seededFocalPoint to the center when none is provided', () => { + lifecycle.assetRequested(BASE_PARAMS); + + expect(store.focalPoint()).toEqual(initialFocalPointState); + expect(store.seededFocalPoint()).toEqual(initialFocalPointState); + }); +}); diff --git a/core-web/libs/image-editor/src/lib/store/features/with-asset.feature.ts b/core-web/libs/image-editor/src/lib/store/features/with-asset.feature.ts index 3f2508c149f..774ec15dee0 100644 --- a/core-web/libs/image-editor/src/lib/store/features/with-asset.feature.ts +++ b/core-web/libs/image-editor/src/lib/store/features/with-asset.feature.ts @@ -9,7 +9,7 @@ import { switchMap } from 'rxjs/operators'; import { ImageEditorState } from '../../models/image-editor.models'; import { DotImageEditorService } from '../../services/dot-image-editor.service'; import { imageEditorLifecycleEvents } from '../image-editor.events'; -import { initialImageEditorState } from '../image-editor.state'; +import { initialFocalPointState, initialImageEditorState } from '../image-editor.state'; import { contextFromParams, errorMessage } from '../image-editor.store-utils'; /** @@ -24,6 +24,10 @@ export function withAsset() { on(imageEditorLifecycleEvents.assetRequested, ({ payload }, _state) => ({ ...initialImageEditorState, assetContext: contextFromParams(payload), + // Seed the focal point from the asset so an existing focal point is + // preserved on Save; the baseline lets a move-and-back read as pristine. + focalPoint: payload.focalPoint ?? initialFocalPointState, + seededFocalPoint: payload.focalPoint ?? initialFocalPointState, previewStatus: 'loading' as const })), on(imageEditorLifecycleEvents.assetLoaded, ({ payload }, state) => ({ diff --git a/core-web/libs/image-editor/src/lib/store/features/with-preview.feature.spec.ts b/core-web/libs/image-editor/src/lib/store/features/with-preview.feature.spec.ts new file mode 100644 index 00000000000..c988692c3ae --- /dev/null +++ b/core-web/libs/image-editor/src/lib/store/features/with-preview.feature.spec.ts @@ -0,0 +1,75 @@ +import { patchState, signalStore, withState } from '@ngrx/signals'; +import { Dispatcher, injectDispatch } from '@ngrx/signals/events'; +import { of } from 'rxjs'; + +import { Injector, runInInjectionContext } from '@angular/core'; +import { TestBed } from '@angular/core/testing'; + +import { withFocalPoint } from './with-focal-point.feature'; +import { withPreview } from './with-preview.feature'; + +import { DotImageEditorService } from '../../services/dot-image-editor.service'; +import { imageEditorToolEvents } from '../image-editor.events'; +import { initialImageEditorState } from '../image-editor.state'; + +// Compose preview + focal point so focal moves can be dispatched and read through +// `isDirty`, which compares the live focal point against the seeded baseline. +const PreviewStore = signalStore( + withState(initialImageEditorState), + withFocalPoint(), + withPreview() +); + +describe('withPreview - isDirty with focal point', () => { + let store: InstanceType; + let tools: ReturnType>; + + beforeEach(() => { + // The debounced `resolveSize$` effect schedules timers; isolate them. + jest.useFakeTimers(); + + TestBed.configureTestingModule({ + providers: [ + PreviewStore, + Dispatcher, + { + provide: DotImageEditorService, + useValue: { getFileSize: jest.fn().mockReturnValue(of(1000)) } + } + ] + }); + const injector = TestBed.inject(Injector); + store = TestBed.inject(PreviewStore); + runInInjectionContext(injector, () => { + tools = injectDispatch(imageEditorToolEvents); + }); + + // Seed a baseline focal point so a move-and-back can be detected as pristine. + patchState(store, { + focalPoint: { x: 0.4, y: 0.6 }, + seededFocalPoint: { x: 0.4, y: 0.6 } + }); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it('is not dirty when the focal point matches the seeded baseline', () => { + expect(store.isDirty()).toBe(false); + }); + + it('is dirty when the focal point moves away from the seeded baseline', () => { + tools.focalPointSet({ x: 0.1, y: 0.9 }); + + expect(store.isDirty()).toBe(true); + }); + + it('is not dirty when the focal point moves and then returns to the seeded baseline', () => { + tools.focalPointSet({ x: 0.1, y: 0.9 }); + expect(store.isDirty()).toBe(true); + + tools.focalPointSet({ x: 0.4, y: 0.6 }); + expect(store.isDirty()).toBe(false); + }); +}); diff --git a/core-web/libs/image-editor/src/lib/store/features/with-preview.feature.ts b/core-web/libs/image-editor/src/lib/store/features/with-preview.feature.ts index aa3bc7242fd..324983eb5db 100644 --- a/core-web/libs/image-editor/src/lib/store/features/with-preview.feature.ts +++ b/core-web/libs/image-editor/src/lib/store/features/with-preview.feature.ts @@ -10,7 +10,11 @@ import { catchError, debounceTime, distinctUntilChanged, switchMap, tap } from ' import { AUTO_PREVIEW_RETRY_LIMIT } from '../../image-editor.constants'; import { ImageEditorState } from '../../models/image-editor.models'; import { DotImageEditorService } from '../../services/dot-image-editor.service'; -import { buildFilterChain, buildPreviewUrl } from '../../utils/image-filter-url.builder'; +import { + buildFilterChain, + buildPreviewUrl, + focalPointsEqual +} from '../../utils/image-filter-url.builder'; import { imageEditorLifecycleEvents } from '../image-editor.events'; /** @@ -102,8 +106,15 @@ export function withPreview() { previewUrl: computed(() => buildPreviewUrl(store.assetContext(), appliedFilters(), store.cacheBust()) ), - /** Whether any edit produces a non-empty filter chain. */ - isDirty: computed(() => appliedFilters().length > 0), + /** + * Whether the editor has unsaved changes: any preview filter, or a + * focal point moved away from the one the asset was opened with. + */ + isDirty: computed( + () => + appliedFilters().length > 0 || + !focalPointsEqual(store.focalPoint(), store.seededFocalPoint()) + ), /** Whether the editor is mid-flight loading a preview. */ isBusy: computed(() => store.previewStatus() === 'loading') }; diff --git a/core-web/libs/image-editor/src/lib/store/features/with-save.feature.spec.ts b/core-web/libs/image-editor/src/lib/store/features/with-save.feature.spec.ts new file mode 100644 index 00000000000..4e724c07d68 --- /dev/null +++ b/core-web/libs/image-editor/src/lib/store/features/with-save.feature.spec.ts @@ -0,0 +1,134 @@ +import { signalStore, signalStoreFeature, type, withComputed, withState } from '@ngrx/signals'; +import { Dispatcher, injectDispatch } from '@ngrx/signals/events'; +import { NEVER, of, throwError } from 'rxjs'; + +import { computed, Injector, runInInjectionContext } from '@angular/core'; +import { TestBed } from '@angular/core/testing'; + +import { DotCMSTempFile } from '@dotcms/dotcms-models'; + +import { withSave } from './with-save.feature'; + +import { AppliedFilter, ImageEditorState } from '../../models/image-editor.models'; +import { DotImageEditorService } from '../../services/dot-image-editor.service'; +import { imageEditorLifecycleEvents } from '../image-editor.events'; +import { initialImageEditorState } from '../image-editor.state'; + +// `withSave` consumes the `appliedFilters` prop from `withPreview`; stub it with a +// fixed empty chain so the feature can be exercised in isolation. +function withAppliedFiltersStub() { + return signalStoreFeature( + type<{ state: ImageEditorState }>(), + withComputed(() => ({ appliedFilters: computed(() => []) })) + ); +} + +// Seed a realistic asset context + a moved focal point so the URL the feature builds +// (store signals -> buildSaveUrl) can be asserted, not only the status transitions. +const SaveStore = signalStore( + withState({ + ...initialImageEditorState, + assetContext: { + ...initialImageEditorState.assetContext, + variable: 'binary', + originalUrl: '/contentAsset/image/abc123/binary' + }, + focalPoint: { x: 0.25, y: 0.75 } + }), + withAppliedFiltersStub(), + withSave() +); + +const TEMP_FILE: DotCMSTempFile = { + fileName: 'edited.png', + folder: 'shared', + id: 'temp_578026b7cc', + image: true, + length: 1024, + mimeType: 'image/png', + referenceUrl: '/dA/temp_578026b7cc', + thumbnailUrl: '/dA/temp_578026b7cc/thumb' +}; + +describe('withSave', () => { + let store: InstanceType; + let service: { saveEditedImage: jest.Mock }; + let lifecycle: ReturnType>; + + function setup(): jest.SpyInstance { + const dispatchSpy = jest.spyOn(Dispatcher.prototype, 'dispatch'); + + TestBed.configureTestingModule({ + providers: [ + SaveStore, + Dispatcher, + { provide: DotImageEditorService, useValue: service } + ] + }); + const injector = TestBed.inject(Injector); + store = TestBed.inject(SaveStore); + runInInjectionContext(injector, () => { + lifecycle = injectDispatch(imageEditorLifecycleEvents); + }); + + return dispatchSpy; + } + + beforeEach(() => { + service = { saveEditedImage: jest.fn().mockReturnValue(of(TEMP_FILE)) }; + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('marks the editor saving on saveRequested while the save is in flight', () => { + // A never-completing save holds the request pending so 'saving' is observable. + service.saveEditedImage.mockReturnValue(NEVER); + setup(); + + lifecycle.saveRequested(); + + expect(store.saveStatus()).toBe('saving'); + expect(store.saveError()).toBeNull(); + }); + + it('dispatches saveSucceeded and returns to idle on success', () => { + const dispatchSpy = setup(); + + lifecycle.saveRequested(); + + expect(service.saveEditedImage).toHaveBeenCalledTimes(1); + expect(dispatchSpy).toHaveBeenCalledWith( + imageEditorLifecycleEvents.saveSucceeded(TEMP_FILE) + ); + expect(store.saveStatus()).toBe('idle'); + expect(store.saveError()).toBeNull(); + }); + + it('builds the Save URL from the asset context, filter chain and focal point', () => { + setup(); + + lifecycle.saveRequested(); + + // assetContext.variable + originalUrl, the (empty) stubbed chain, and the moved + // focal point all flow through buildSaveUrl into the single GET. + expect(service.saveEditedImage).toHaveBeenCalledWith( + '/contentAsset/image/abc123/binary/filter/FocalPoint/fp/0.25,0.75' + + '?binaryFieldId=binary&_imageToolSaveFile=true&overwrite=true' + ); + }); + + it('dispatches saveFailed and surfaces the error on failure', () => { + service.saveEditedImage.mockReturnValue(throwError(() => new Error('boom'))); + const dispatchSpy = setup(); + + lifecycle.saveRequested(); + + expect(dispatchSpy).toHaveBeenCalledWith( + imageEditorLifecycleEvents.saveFailed(expect.any(Error)) + ); + expect(store.saveStatus()).toBe('error'); + expect(store.saveError()).toBe('boom'); + }); +}); diff --git a/core-web/libs/image-editor/src/lib/store/features/with-save.feature.ts b/core-web/libs/image-editor/src/lib/store/features/with-save.feature.ts new file mode 100644 index 00000000000..362373d2fab --- /dev/null +++ b/core-web/libs/image-editor/src/lib/store/features/with-save.feature.ts @@ -0,0 +1,77 @@ +import { tapResponse } from '@ngrx/operators'; +import { signalStoreFeature, type } from '@ngrx/signals'; +import { Dispatcher, Events, on, withEventHandlers, withReducer } from '@ngrx/signals/events'; + +import { inject, Signal } from '@angular/core'; + +import { switchMap } from 'rxjs/operators'; + +import { AppliedFilter, ImageEditorState } from '../../models/image-editor.models'; +import { DotImageEditorService } from '../../services/dot-image-editor.service'; +import { buildSaveUrl } from '../../utils/image-filter-url.builder'; +import { imageEditorLifecycleEvents } from '../image-editor.events'; +import { errorMessage } from '../image-editor.store-utils'; + +/** + * Save feature: stages the current edits (preview filter chain + focal point) into a + * temp file. A `saveRequested` marks the editor saving; the `save$` effect GETs the + * Save URL and folds the resulting temp file (`saveSucceeded`) or error (`saveFailed`) + * back in. Consumes the `appliedFilters` selector from {@link withPreview}, so it + * composes after it. + */ +export function withSave() { + return signalStoreFeature( + type<{ state: ImageEditorState; props: { appliedFilters: Signal } }>(), + withReducer( + on(imageEditorLifecycleEvents.saveRequested, (_event, state) => ({ + ...state, + saveStatus: 'saving' as const, + saveError: null + })), + on(imageEditorLifecycleEvents.saveSucceeded, (_event, state) => ({ + ...state, + saveStatus: 'idle' as const + })), + on(imageEditorLifecycleEvents.saveFailed, ({ payload }, state) => ({ + ...state, + saveStatus: 'error' as const, + saveError: errorMessage(payload, 'Failed to save image') + })) + ), + withEventHandlers((store) => { + const events = inject(Events); + const dispatcher = inject(Dispatcher); + const service = inject(DotImageEditorService); + + return { + // Save the edited image whenever the user requests it. + save$: events.on(imageEditorLifecycleEvents.saveRequested).pipe( + switchMap(() => + service + .saveEditedImage( + buildSaveUrl( + store.assetContext(), + store.appliedFilters(), + store.focalPoint(), + store.seededFocalPoint(), + store.assetContext().variable + ) + ) + .pipe( + tapResponse({ + next: (tempFile) => + dispatcher.dispatch( + imageEditorLifecycleEvents.saveSucceeded(tempFile) + ), + error: (error) => + dispatcher.dispatch( + imageEditorLifecycleEvents.saveFailed(error) + ) + }) + ) + ) + ) + }; + }) + ); +} diff --git a/core-web/libs/image-editor/src/lib/store/image-editor.events.ts b/core-web/libs/image-editor/src/lib/store/image-editor.events.ts index 203d1e0b3fd..920a85fd038 100644 --- a/core-web/libs/image-editor/src/lib/store/image-editor.events.ts +++ b/core-web/libs/image-editor/src/lib/store/image-editor.events.ts @@ -1,6 +1,8 @@ import { type } from '@ngrx/signals'; import { eventGroup } from '@ngrx/signals/events'; +import { DotCMSTempFile } from '@dotcms/dotcms-models'; + import { ActiveTool, CompressionMode, @@ -71,7 +73,7 @@ export const imageEditorHistoryEvents = eventGroup({ } }); -/** Events covering the editor lifecycle: load, preview and download. */ +/** Events covering the editor lifecycle: load, preview, download and save. */ export const imageEditorLifecycleEvents = eventGroup({ source: 'Image Editor Lifecycle', events: { @@ -80,6 +82,9 @@ export const imageEditorLifecycleEvents = eventGroup({ previewErrored: type(), retryRequested: type(), downloadRequested: type(), + saveRequested: type(), + saveSucceeded: type(), + saveFailed: type(), assetLoaded: type<{ naturalWidth: number; naturalHeight: number; diff --git a/core-web/libs/image-editor/src/lib/store/image-editor.state.ts b/core-web/libs/image-editor/src/lib/store/image-editor.state.ts index 2c6afa0a0b9..47f04c250ad 100644 --- a/core-web/libs/image-editor/src/lib/store/image-editor.state.ts +++ b/core-web/libs/image-editor/src/lib/store/image-editor.state.ts @@ -80,9 +80,12 @@ export const initialImageEditorState: ImageEditorState = { fileInfo: initialFileInfoState, zoom: initialZoomState, focalPoint: initialFocalPointState, + seededFocalPoint: initialFocalPointState, activeTool: 'move', previewStatus: 'idle', previewRetries: 0, + saveStatus: 'idle', + saveError: null, error: null, history: [], historyIndex: -1, diff --git a/core-web/libs/image-editor/src/lib/store/image-editor.store.ts b/core-web/libs/image-editor/src/lib/store/image-editor.store.ts index b15eb74bf76..aafef6f9688 100644 --- a/core-web/libs/image-editor/src/lib/store/image-editor.store.ts +++ b/core-web/libs/image-editor/src/lib/store/image-editor.store.ts @@ -9,6 +9,7 @@ import { withFocalPoint, withHistory, withPreview, + withSave, withTransform, withView } from './features'; @@ -21,9 +22,9 @@ import { initialImageEditorState } from './image-editor.state'; * single place (`features/with-*.feature.ts`). * * Order matters only where features consume each other's selectors: `withPreview` - * derives `previewUrl`, which `withDownload` reads — so download composes after - * preview. The store is NOT provided in root; the editor dialog supplies it so - * each editor instance is isolated. + * derives `previewUrl`/`appliedFilters`, which `withDownload` and `withSave` read — + * so download and save compose after preview. The store is NOT provided in root; the + * editor dialog supplies it so each editor instance is isolated. */ export const ImageEditorStore = signalStore( withState(initialImageEditorState), @@ -36,5 +37,6 @@ export const ImageEditorStore = signalStore( withHistory(), withAsset(), withPreview(), - withDownload() + withDownload(), + withSave() ); diff --git a/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.spec.ts b/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.spec.ts index 73c5440ff80..a3691677535 100644 --- a/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.spec.ts +++ b/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.spec.ts @@ -1,11 +1,19 @@ -import { buildFilterChain, buildPreviewUrl, cleanUrl, toHsb } from './image-filter-url.builder'; +import { + buildFilterChain, + buildPreviewUrl, + buildSaveUrl, + cleanUrl, + toHsb +} from './image-filter-url.builder'; import { AdjustState, + AppliedFilter, CompressionMode, CropState, FileInfoState, ImageEditorAssetContext, + NormalizedPoint, TransformState } from '../models/image-editor.models'; @@ -319,4 +327,77 @@ describe('image-filter-url.builder', () => { expect(url).toBe('/contentAsset/image/abc123/fileAsset?test=5'); }); }); + + describe('buildSaveUrl', () => { + const center: NormalizedPoint = { x: 0.5, y: 0.5 }; + + it('appends the save flags to the base url for an empty chain and centered focal', () => { + const url = buildSaveUrl(baseContext, [], center, center, 'fileAsset'); + expect(url).toBe( + '/contentAsset/image/abc123/fileAsset?binaryFieldId=fileAsset&_imageToolSaveFile=true' + ); + }); + + it('keeps the filter segment and adds no overwrite when only filters are applied', () => { + const filters = chain({ adjust: { grayscale: true } }); + const url = buildSaveUrl(baseContext, filters, center, center, 'fileAsset'); + expect(url).toBe( + '/contentAsset/image/abc123/fileAsset/filter/Grayscale/grayscale/1' + + '?binaryFieldId=fileAsset&_imageToolSaveFile=true' + ); + expect(url).not.toContain('overwrite'); + expect(url).not.toContain('FocalPoint'); + }); + + it('does not inject a FocalPoint filter for an untouched centered focal point', () => { + const url = buildSaveUrl(baseContext, [], center, center, 'fileAsset'); + expect(url).not.toContain('FocalPoint'); + expect(url).not.toContain('overwrite'); + }); + + it('injects a FocalPoint filter and overwrite flag when the focal point moved from centre', () => { + const url = buildSaveUrl(baseContext, [], { x: 0.25, y: 0.75 }, center, 'fileAsset'); + expect(url).toBe( + '/contentAsset/image/abc123/fileAsset/filter/FocalPoint/fp/0.25,0.75' + + '?binaryFieldId=fileAsset&_imageToolSaveFile=true&overwrite=true' + ); + }); + + it('re-stages an untouched non-centre focal point so it is preserved on save', () => { + const seeded: NormalizedPoint = { x: 0.3, y: 0.7 }; + const url = buildSaveUrl(baseContext, [], seeded, seeded, 'fileAsset'); + expect(url).toContain('/filter/FocalPoint/fp/0.3,0.7'); + expect(url).toContain('&overwrite=true'); + }); + + it('sends the focal point when recentring a previously-set focal point', () => { + // Opened at (0.3,0.7), dragged back to centre: must still send so the server overwrites. + const url = buildSaveUrl(baseContext, [], center, { x: 0.3, y: 0.7 }, 'fileAsset'); + expect(url).toContain('/filter/FocalPoint/fp/0.5,0.5'); + expect(url).toContain('&overwrite=true'); + }); + + it('treats a sub-epsilon nudge back to the seeded centre as unchanged (no focal entry)', () => { + const nudged: NormalizedPoint = { x: 0.5 + 1e-6, y: 0.5 - 1e-6 }; + const url = buildSaveUrl(baseContext, [], nudged, center, 'fileAsset'); + expect(url).not.toContain('FocalPoint'); + expect(url).not.toContain('overwrite'); + }); + + it('appends FocalPoint to an existing chain as a single comma-joined filter segment', () => { + const filters: AppliedFilter[] = chain({ + crop: { active: true, x: 0, y: 0, w: 100, h: 50 } + }); + const url = buildSaveUrl(baseContext, filters, { x: 0.1, y: 0.2 }, center, 'fileAsset'); + expect(url).toContain( + '/filter/Crop,FocalPoint/crop_w/100/crop_h/50/crop_x/0/crop_y/0/fp/0.1,0.2' + ); + expect(url).toContain('&overwrite=true'); + }); + + it('includes the binaryFieldId in the query', () => { + const url = buildSaveUrl(baseContext, [], center, center, 'myImage'); + expect(url).toContain('binaryFieldId=myImage'); + }); + }); }); diff --git a/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.ts b/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.ts index bd7abd16d82..5edd725887f 100644 --- a/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.ts +++ b/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.ts @@ -4,7 +4,8 @@ import { AppliedFilter, CompressionMode, FilterChainInput, - ImageEditorAssetContext + ImageEditorAssetContext, + NormalizedPoint } from '../models/image-editor.models'; /** @@ -176,3 +177,75 @@ export function buildPreviewUrl( return url; } + +/** + * Compares two normalized focal points within a small tolerance. Focal coordinates are + * floats (pointer math and 0.01 keyboard nudges that never round), so an exact `===` would + * report a point nudged back to its origin as still-moved — leaving `isDirty` stuck true and + * the discard guard armed, and the Save URL emitting a focal entry for a visually-unchanged point. + * @param a - First focal point + * @param b - Second focal point + * @param epsilon - Maximum per-axis difference treated as equal (default 1e-4) + * @returns Whether the two points are equal within the tolerance + */ +export function focalPointsEqual(a: NormalizedPoint, b: NormalizedPoint, epsilon = 1e-4): boolean { + return Math.abs(a.x - b.x) < epsilon && Math.abs(a.y - b.y) < epsilon; +} + +/** The neutral, "no focal point set" position the editor opens with by default. */ +const CENTER_FOCAL_POINT: NormalizedPoint = { x: 0.5, y: 0.5 }; + +/** + * Builds the Save URL: the same `/filter/` segment the preview uses, but + * without the cache-bust and with the `_imageToolSaveFile` flag so the servlet + * stages the filtered render into a temp file (returned as a {@link DotCMSTempFile}). + * + * The focal point is folded in as a trailing `FocalPoint` chain entry (the registry + * resolves the name case-insensitively) and forces `overwrite=true` — which the + * `FocalPointImageFilter` requires to stage the focal metadata onto the result — whenever + * it holds a real (non-centre) value OR differs from the point the asset opened with (so + * recentring a previously-set focal point still overwrites it, and an existing focal point + * is re-staged onto the new temp rather than dropped). It is intentionally NOT part of the + * preview chain, so it only ever reaches the server on Save. + * @param ctx - Resolved asset context (provides the base URL) + * @param chain - The ordered preview filters produced by {@link buildFilterChain} + * @param focalPoint - The current normalized 0..1 focal point + * @param seededFocalPoint - The focal point the asset was opened with (the dirty baseline) + * @param binaryFieldId - The binary field variable (sent for legacy parity) + * @returns The fully-qualified Save URL + */ +export function buildSaveUrl( + ctx: ImageEditorAssetContext, + chain: AppliedFilter[], + focalPoint: NormalizedPoint, + seededFocalPoint: NormalizedPoint, + binaryFieldId: string +): string { + // Persist the focal point (and force the server to commit it via `overwrite`) when it + // holds a real, non-centre value OR the user moved it from what the asset opened with + // (the latter also covers recentring a previously-set focal point back to the middle). + const focalApplied = + !focalPointsEqual(focalPoint, seededFocalPoint) || + !focalPointsEqual(focalPoint, CENTER_FOCAL_POINT); + const saveChain: AppliedFilter[] = focalApplied + ? [...chain, { name: 'FocalPoint', args: `/fp/${focalPoint.x},${focalPoint.y}` }] + : chain; + + let url = ctx.originalUrl; + + if (saveChain.length > 0) { + const names = saveChain.map((filter) => filter.name).join(','); + const args = saveChain.map((filter) => filter.args).join(''); + url = `${url}/filter/${names}${args}`; + } + + url = cleanUrl(url); + url += url.includes('?') ? '&' : '?'; + url += `binaryFieldId=${binaryFieldId}&_imageToolSaveFile=true`; + + if (focalApplied) { + url += '&overwrite=true'; + } + + return url; +} diff --git a/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties b/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties index 0e0f1827099..f9c238c1118 100644 --- a/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties +++ b/dotCMS/src/main/webapp/WEB-INF/messages/Language.properties @@ -6476,6 +6476,8 @@ edit.content.image-editor.close.aria=Close image editor edit.content.image-editor.footer.cancel=Cancel edit.content.image-editor.footer.download=Download edit.content.image-editor.footer.download.aria=Download image +edit.content.image-editor.footer.save=Save +edit.content.image-editor.footer.save.aria=Save edited image edit.content.image-editor.canvas.alt=Image preview edit.content.image-editor.canvas.loading=Applying preview… edit.content.image-editor.canvas.error.title=Could not load image From 96414df3257cf75cc7aa8c4a16d45df92c0ed36c Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Mon, 29 Jun 2026 11:00:44 -0400 Subject: [PATCH 02/18] feat(edit-content): seed focal on open and tolerate metadata-less temp in binary field onEditImage reads the asset's stored focal point ({field}MetaData.focalPoint) and passes it to the launcher so the marker re-seeds on reopen. Guard the null metadata that image-tool temp files return so the field renders instead of crashing. --- ...dot-edit-content-binary-field.component.ts | 11 ++++- .../store/binary-field.store.spec.ts | 20 ++++++++ .../store/binary-field.store.ts | 5 +- .../utils/binary-field-utils.spec.ts | 48 +++++++++++++++++++ .../utils/binary-field-utils.ts | 31 ++++++++++++ 5 files changed, 112 insertions(+), 3 deletions(-) create mode 100644 core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/utils/binary-field-utils.spec.ts diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/dot-edit-content-binary-field.component.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/dot-edit-content-binary-field.component.ts index 1b4d3a156b1..dea1507cc92 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/dot-edit-content-binary-field.component.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/dot-edit-content-binary-field.component.ts @@ -60,7 +60,7 @@ import { BinaryFieldMode, BinaryFieldStatus } from './interfaces'; import { DotBinaryFieldEditImageService } from './service/dot-binary-field-edit-image/dot-binary-field-edit-image.service'; import { DotBinaryFieldValidatorService } from './service/dot-binary-field-validator/dot-binary-field-validator.service'; import { DotBinaryFieldStore } from './store/binary-field.store'; -import { getFileMetadata, getUiMessage } from './utils/binary-field-utils'; +import { getFileMetadata, getUiMessage, parseFocalPoint } from './utils/binary-field-utils'; import { DEFAULT_MONACO_CONFIG } from '../../models/dot-edit-content-field.constant'; import { getFieldVariablesParsed, stringToJson } from '../../utils/functions.util'; @@ -426,6 +426,12 @@ export class DotEditContentBinaryFieldComponent : null; const fieldName = this.$field()?.name; const variable = this.variable; + // Seed the editor with the asset's stored focal point (exposed on the binary + // field metadata as a `"x,y"` string) so reopening restores the marker instead + // of resetting it to centre. + const focalPoint = parseFocalPoint( + (metadata as { focalPoint?: string } | null)?.focalPoint + ); // The launcher contract requires a resolved field/variable; the image-editor lib // treats them as guaranteed strings. Bail (rather than leak `undefined`) if the @@ -444,7 +450,8 @@ export class DotEditContentBinaryFieldComponent fieldName, byInode: !!inode, fileName: this.contentlet?.fileName ?? metadata?.name, - mimeType: metadata?.contentType + mimeType: metadata?.contentType, + focalPoint }) .pipe( filter((tempFile): tempFile is DotCMSTempFile => !!tempFile), diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/store/binary-field.store.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/store/binary-field.store.spec.ts index e6748b8f3db..da66e8c1255 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/store/binary-field.store.spec.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/store/binary-field.store.spec.ts @@ -270,6 +270,26 @@ describe('DotBinaryFieldStore', () => { done(); }); }); + + it('should handle a temp file with null metadata (image-editor save) without throwing', (done) => { + // Temp files minted by the image-editor save (`_imageToolSaveFile`) come back + // with `metadata: null`; the store must stage them instead of crashing. + store.setFileFromTemp({ + ...TEMP_FILE_MOCK, + metadata: null + }); + + store.state$.subscribe((state) => { + httpMock.expectNone(TEMP_FILE_MOCK.referenceUrl, HttpMethod.GET); + expect(state.status).toBe(BinaryFieldStatus.PREVIEW); + expect(state.tempFile).toEqual({ + ...TEMP_FILE_MOCK, + metadata: null, + content: '' + }); + done(); + }); + }); }); }); }); diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/store/binary-field.store.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/store/binary-field.store.ts index ad9ca2e2b14..d5892a63b26 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/store/binary-field.store.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/store/binary-field.store.ts @@ -212,7 +212,10 @@ export class DotBinaryFieldStore extends ComponentStore { private handleTempFile(tempFile: DotCMSTempFile): Observable { const { referenceUrl, metadata } = tempFile; - const { editableAsText = false } = metadata; + // Temp files minted by the image editor save (`_imageToolSaveFile`) come back with + // `metadata: null` (the servlet builds the metadata from the empty temp before copying + // the filtered bytes), so guard the destructure — `= false` only defaults `undefined`. + const { editableAsText = false } = metadata ?? {}; const obs$ = editableAsText ? this.getFileContent(referenceUrl) : of(''); diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/utils/binary-field-utils.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/utils/binary-field-utils.spec.ts new file mode 100644 index 00000000000..3bc8fccf120 --- /dev/null +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/utils/binary-field-utils.spec.ts @@ -0,0 +1,48 @@ +import { getFileMetadata, parseFocalPoint } from './binary-field-utils'; + +describe('binary-field-utils', () => { + describe('parseFocalPoint', () => { + it('parses a "x,y" focal point string into a point', () => { + expect(parseFocalPoint('0.88,0.31')).toEqual({ x: 0.88, y: 0.31 }); + }); + + it('returns undefined for an unset/empty value', () => { + expect(parseFocalPoint(undefined)).toBeUndefined(); + expect(parseFocalPoint(null)).toBeUndefined(); + expect(parseFocalPoint('')).toBeUndefined(); + }); + + it('treats the backend "no focal point" sentinels (0,0) as undefined', () => { + // The backend uses "0.0"/"0,0"/(0,0) to mean "no focal point" -> editor opens centred. + expect(parseFocalPoint('0,0')).toBeUndefined(); + expect(parseFocalPoint('0.0')).toBeUndefined(); + }); + + it('returns undefined for a malformed value', () => { + expect(parseFocalPoint('not-a-point')).toBeUndefined(); + expect(parseFocalPoint('abc,xyz')).toBeUndefined(); + }); + }); + + describe('getFileMetadata', () => { + it('returns an empty object when the contentlet is null', () => { + // The binary-field preview falls back here for image-editor temp files + // (which carry no contentlet and `metadata: null`), so it must not throw. + expect(getFileMetadata(null as never)).toEqual({}); + }); + + it('returns the contentlet metaData when present', () => { + const metaData = { isImage: true, contentType: 'image/png' }; + + expect(getFileMetadata({ metaData } as never)).toBe(metaData); + }); + + it('falls back to the {fieldVariable}MetaData entry', () => { + const assetMetaData = { isImage: true }; + + expect(getFileMetadata({ fieldVariable: 'asset', assetMetaData } as never)).toBe( + assetMetaData + ); + }); + }); +}); diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/utils/binary-field-utils.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/utils/binary-field-utils.ts index bb216eb7eac..2c0dd501c48 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/utils/binary-field-utils.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/utils/binary-field-utils.ts @@ -38,6 +38,12 @@ export const getUiMessage = (messageKey: string, ...args: string[]): UiMessageI }; export const getFileMetadata = (contentlet: DotCMSContentlet) => { + // The preview falls back here when a temp file has no metadata (image-editor saves + // come back with `metadata: null` and a null contentlet), so guard the destructure. + if (!contentlet) { + return {}; + } + const { metaData, fieldVariable } = contentlet; const metadata = metaData || contentlet[`${fieldVariable}MetaData`]; @@ -50,3 +56,28 @@ export const getFieldVersion = (contentlet: DotCMSContentlet) => { return fileAssetVersion || contentlet[`${fieldVariable}Version`]; }; + +/** + * Parses a focal point string from a binary field's metadata (`"x,y"`, normalized 0..1) + * into a point the image editor can seed its marker with. The backend uses `"0.0"` / `(0,0)` + * to mean "no focal point", so that — like an absent or invalid value — yields `undefined`, + * letting the editor open centred. + * + * @param value The raw `focalPoint` metadata value, e.g. `"0.88,0.31"` + * @returns The parsed point, or `undefined` when unset/invalid + */ +export const parseFocalPoint = ( + value: string | undefined | null +): { x: number; y: number } | undefined => { + if (!value) { + return undefined; + } + + const [x, y] = value.split(',').map(Number); + + if (!Number.isFinite(x) || !Number.isFinite(y) || (x === 0 && y === 0)) { + return undefined; + } + + return { x, y }; +}; From dedc5f436e87d35cae397bf0f390a2b29755911d Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Mon, 29 Jun 2026 11:00:44 -0400 Subject: [PATCH 03/18] fix(image): populate temp metadata on image-tool save and expose focalPoint on read The _imageToolSaveFile save block created the temp from the empty file, so it came back with metadata:null/image:false; re-wrap it after copying the rendered bytes so it carries real metadata (mirrors TempFileAPI.createTempFile). Expose focalPoint on DefaultTransformStrategy.addBinaries (mirrors BinaryViewStrategy) so the editor can re-seed the marker on reopen. --- .../strategy/DefaultTransformStrategy.java | 10 +- .../servlets/BinaryExporterServlet.java | 9 +- .../DefaultTransformStrategyTest.java | 117 ++++++++++++++++++ 3 files changed, 134 insertions(+), 2 deletions(-) create mode 100644 dotCMS/src/test/java/com/dotmarketing/portlets/contentlet/transform/strategy/DefaultTransformStrategyTest.java diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/transform/strategy/DefaultTransformStrategy.java b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/transform/strategy/DefaultTransformStrategy.java index f4a40019253..4d6b4be0364 100644 --- a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/transform/strategy/DefaultTransformStrategy.java +++ b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/transform/strategy/DefaultTransformStrategy.java @@ -14,6 +14,7 @@ import com.dotmarketing.business.IdentifierAPI; import com.dotmarketing.exception.DotDataException; import com.dotmarketing.exception.DotSecurityException; +import com.dotmarketing.image.focalpoint.FocalPointAPI; import com.dotmarketing.portlets.categories.model.Category; import com.dotmarketing.portlets.contentlet.model.Contentlet; import com.dotmarketing.portlets.contentlet.model.ContentletVersionInfo; @@ -264,9 +265,16 @@ private void addBinaries(final Contentlet contentlet, final Map if (null != metadata) { Map metaMap = new HashMap<>(metadata.getMap()); metaMap.remove("path"); + // Focal point lives in the binary's custom metadata under a prefixed key, so it + // is not exposed under the clean "focalPoint" key by metadata.getMap(). Surface + // it explicitly so the image editor can re-seed its marker when reopened. + final String focalPoint = Try.of(() -> + metadata.getCustomMeta().getOrDefault(FocalPointAPI.FOCAL_POINT, "0.0").toString() + ).getOrElse("0.0"); + metaMap.put(FocalPointAPI.FOCAL_POINT, focalPoint); map.put(velocityVarName + "MetaData", metaMap); putBinaryLinks(velocityVarName, metadata.getName(), contentlet, map); - } + } } catch (final Exception e) { Logger.warn(this, String.format("An error occurred when retrieving the Binary file from field" diff --git a/dotCMS/src/main/java/com/dotmarketing/servlets/BinaryExporterServlet.java b/dotCMS/src/main/java/com/dotmarketing/servlets/BinaryExporterServlet.java index 5be5ac02c95..9e0ae611951 100644 --- a/dotCMS/src/main/java/com/dotmarketing/servlets/BinaryExporterServlet.java +++ b/dotCMS/src/main/java/com/dotmarketing/servlets/BinaryExporterServlet.java @@ -412,10 +412,17 @@ public void doGet(HttpServletRequest req, HttpServletResponse resp) throws Servl // THIS IS WHERE THE MAGIC HAPPENS // this creates a temp resource using the altered file if (req.getParameter(WebKeys.IMAGE_TOOL_SAVE_FILES) != null && user!=null && !user.equals(APILocator.getUserAPI().getAnonymousUser())) { - final DotTempFile temp = tempFileAPI.createEmptyTempFile(inputFile.getName(), req); + DotTempFile temp = tempFileAPI.createEmptyTempFile(inputFile.getName(), req); FileUtil.copyFile(data.getDataFile(), temp.file); //Temp files time-mark must be updated so they can be recognized by the tempFileAPI temp.file.setLastModified(System.currentTimeMillis()); + // createEmptyTempFile derives metadata from the still-empty file, so the rendered + // image would otherwise come back as metadata:null/image:false/mimeType:unknown. + // Re-wrap to regenerate metadata from the now-populated file (mirrors + // TempFileAPI.createTempFile) so consumers get a usable image preview. + if (temp.metadata == null && temp.file.exists()) { + temp = new DotTempFile(temp.id, temp.file); + } copyMetadata(uuid, fieldVarName, temp); resp.getWriter().println(DotObjectMapperProvider.getInstance().getDefaultObjectMapper().writeValueAsString(temp)); resp.getWriter().close(); diff --git a/dotCMS/src/test/java/com/dotmarketing/portlets/contentlet/transform/strategy/DefaultTransformStrategyTest.java b/dotCMS/src/test/java/com/dotmarketing/portlets/contentlet/transform/strategy/DefaultTransformStrategyTest.java new file mode 100644 index 00000000000..e4a202d1356 --- /dev/null +++ b/dotCMS/src/test/java/com/dotmarketing/portlets/contentlet/transform/strategy/DefaultTransformStrategyTest.java @@ -0,0 +1,117 @@ +package com.dotmarketing.portlets.contentlet.transform.strategy; + +import static com.dotmarketing.portlets.contentlet.transform.strategy.TransformOptions.BINARIES; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import com.dotcms.api.APIProvider; +import com.dotcms.contenttype.model.field.BinaryField; +import com.dotcms.contenttype.model.field.Field; +import com.dotcms.contenttype.model.type.ContentType; +import com.dotcms.storage.model.Metadata; +import com.dotmarketing.image.focalpoint.FocalPointAPI; +import com.dotmarketing.portlets.contentlet.model.Contentlet; +import java.io.Serializable; +import java.lang.reflect.Method; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import org.junit.Test; +import org.mockito.Mockito; + +public class DefaultTransformStrategyTest { + + private static final String FIELD_VAR = "fileField"; + private static final String META_KEY = FIELD_VAR + "MetaData"; + + /** + * Custom metadata is persisted in the backing map under the {@link Metadata#CUSTOM_PROP_PREFIX} + * ("dot:") prefix; {@code Metadata.getCustomMeta()} strips that prefix when exposing it. + */ + private static final String CUSTOM_FOCAL_POINT_KEY = + Metadata.CUSTOM_PROP_PREFIX + FocalPointAPI.FOCAL_POINT; + + /** + * Invokes the private {@code addBinaries} method in isolation so the focal-point behavior can be + * exercised without standing up the rest of the transform pipeline. + */ + @SuppressWarnings("unchecked") + private void invokeAddBinaries(final DefaultTransformStrategy strategy, + final Contentlet contentlet, final Map map) throws Exception { + final Method addBinaries = DefaultTransformStrategy.class.getDeclaredMethod( + "addBinaries", Contentlet.class, Map.class, Set.class); + addBinaries.setAccessible(true); + addBinaries.invoke(strategy, contentlet, map, Set.of(BINARIES)); + } + + private Contentlet mockContentletWithBinary(final Metadata metadata) throws Exception { + final Field field = Mockito.mock(BinaryField.class); + Mockito.when(field.variable()).thenReturn(FIELD_VAR); + + final ContentType contentType = Mockito.mock(ContentType.class); + Mockito.when(contentType.fields(BinaryField.class)).thenReturn(List.of(field)); + + final Contentlet contentlet = Mockito.mock(Contentlet.class); + Mockito.when(contentlet.getContentType()).thenReturn(contentType); + Mockito.when(contentlet.isFileAsset()).thenReturn(false); + Mockito.when(contentlet.getIdentifier()).thenReturn("identifier-1"); + Mockito.when(contentlet.getInode()).thenReturn("inode-1"); + Mockito.when(contentlet.getBinaryMetadata(FIELD_VAR)).thenReturn(metadata); + return contentlet; + } + + /** + * When the binary's custom metadata carries a focal point, it must be surfaced under + * {@code {field}MetaData.focalPoint} on the REST read path so the image editor can re-seed its + * marker. Regression coverage for #36067. + */ + @Test + public void testAddBinaries_whenCustomMetaHasFocalPoint_surfacesItInMetaDataMap() + throws Exception { + + final Map fieldsMeta = new HashMap<>(); + fieldsMeta.put("name", "image.png"); + fieldsMeta.put(CUSTOM_FOCAL_POINT_KEY, "0.25,0.75"); + final Metadata metadata = new Metadata(FIELD_VAR, fieldsMeta); + + final APIProvider toolBox = Mockito.mock(APIProvider.class); + final DefaultTransformStrategy strategy = new DefaultTransformStrategy(toolBox); + final Contentlet contentlet = mockContentletWithBinary(metadata); + + final Map map = new HashMap<>(); + invokeAddBinaries(strategy, contentlet, map); + + assertTrue("Expected the field MetaData entry to be present", map.containsKey(META_KEY)); + @SuppressWarnings("unchecked") + final Map metaMap = (Map) map.get(META_KEY); + assertEquals("Focal point from custom metadata must be exposed under the focalPoint key", + "0.25,0.75", metaMap.get(FocalPointAPI.FOCAL_POINT)); + } + + /** + * When the binary has no focal point in its custom metadata, the read path must default the + * {@code focalPoint} entry to "0.0" rather than omit it, matching the GraphQL/Velocity view. + */ + @Test + public void testAddBinaries_whenCustomMetaHasNoFocalPoint_defaultsToZero() + throws Exception { + + final Map fieldsMeta = new HashMap<>(); + fieldsMeta.put("name", "image.png"); + final Metadata metadata = new Metadata(FIELD_VAR, fieldsMeta); + + final APIProvider toolBox = Mockito.mock(APIProvider.class); + final DefaultTransformStrategy strategy = new DefaultTransformStrategy(toolBox); + final Contentlet contentlet = mockContentletWithBinary(metadata); + + final Map map = new HashMap<>(); + invokeAddBinaries(strategy, contentlet, map); + + assertTrue("Expected the field MetaData entry to be present", map.containsKey(META_KEY)); + @SuppressWarnings("unchecked") + final Map metaMap = (Map) map.get(META_KEY); + assertEquals("focalPoint must default to 0.0 when absent from custom metadata", + "0.0", metaMap.get(FocalPointAPI.FOCAL_POINT)); + } +} From f90d99105317e409a80756c331942e37fd7ac29b Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Mon, 29 Jun 2026 14:58:16 -0400 Subject: [PATCH 04/18] fix(edit-content): seed image editor focal point from the field metadata on open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit onEditImage called getFileMetadata(this.contentlet), but the raw @Input contentlet carries no fieldVariable, so it could not resolve {field}MetaData and returned {} — dropping the stored focalPoint. Pass fieldVariable explicitly (as the preview does) so the editor seeds the saved focal point instead of resetting to centre. --- ...dit-content-binary-field.component.spec.ts | 38 +++++++++++++++++++ ...dot-edit-content-binary-field.component.ts | 9 ++++- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/dot-edit-content-binary-field.component.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/dot-edit-content-binary-field.component.spec.ts index 0aae7e22c71..6a292f94580 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/dot-edit-content-binary-field.component.spec.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/dot-edit-content-binary-field.component.spec.ts @@ -340,6 +340,44 @@ describe('DotEditContentBinaryFieldComponent', () => { ); }); + it('should seed the editor with the stored focal point from the field metadata', () => { + spectator.setInput('contentlet', { + ...MOCK_DOTCMS_FILE, + inode: 'inode-123', + metaData: undefined, + [`${BINARY_FIELD_MOCK.variable}MetaData`]: { + ...fileMetaData, + focalPoint: '0.2,0.7' + } + }); + spectator.detectChanges(); + + spectator.triggerEventHandler(DotBinaryFieldPreviewComponent, 'editImage', null); + + expect(imageEditorLauncherMock.open).toHaveBeenCalledWith( + expect.objectContaining({ focalPoint: { x: 0.2, y: 0.7 } }) + ); + }); + + it('should not seed a focal point when the field metadata has none', () => { + spectator.setInput('contentlet', { + ...MOCK_DOTCMS_FILE, + inode: 'inode-123', + metaData: undefined, + [`${BINARY_FIELD_MOCK.variable}MetaData`]: { + ...fileMetaData, + focalPoint: '0.0' + } + }); + spectator.detectChanges(); + + spectator.triggerEventHandler(DotBinaryFieldPreviewComponent, 'editImage', null); + + expect(imageEditorLauncherMock.open).toHaveBeenCalledWith( + expect.objectContaining({ focalPoint: undefined }) + ); + }); + it('should apply the edited temp file through the store', () => { imageEditorLauncherMock.open.mockReturnValue(of(TEMP_FILE_MOCK)); const spyTempFile = jest.spyOn(store, 'setFileFromTemp'); diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/dot-edit-content-binary-field.component.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/dot-edit-content-binary-field.component.ts index dea1507cc92..e9d0c137472 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/dot-edit-content-binary-field.component.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/dot-edit-content-binary-field.component.ts @@ -421,8 +421,15 @@ export class DotEditContentBinaryFieldComponent } const inode = this.contentlet?.inode; + // `getFileMetadata` resolves `{fieldVariable}MetaData` off the contentlet, but the raw + // @Input contentlet has no `fieldVariable` — the field var lives in `this.variable`. Pass + // it explicitly (as the preview does via setFileFromContentlet) so the field's metadata + // (and its `focalPoint`) is actually found instead of falling back to {}. const metadata = this.contentlet - ? (getFileMetadata(this.contentlet) as Partial) + ? (getFileMetadata({ + ...this.contentlet, + fieldVariable: this.variable + }) as Partial) : null; const fieldName = this.$field()?.name; const variable = this.variable; From b143fd54e221f2fbae42811c89bab794fb84af84 Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Mon, 29 Jun 2026 15:05:03 -0400 Subject: [PATCH 05/18] fix(image-editor): red focal marker and keep it aligned on dialog resize - Make the focal target dot red (var(--p-red-500)) instead of the UI primary blue so it reads as a focal marker and contrasts against arbitrary image content. - Observe the stage (the image's offsetParent) in addition to the image: toggling full-screen re-centres the image without changing its own size, which a ResizeObserver on the image alone never reports, leaving imageRect.x/y stale and the focal/crop overlays mispositioned on the full-screen -> windowed transition. --- .../dot-image-editor-canvas.component.ts | 15 ++++++++++++++- .../dot-image-editor-focal-overlay.component.scss | 6 ++++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/core-web/libs/image-editor/src/lib/components/dot-image-editor-canvas/dot-image-editor-canvas.component.ts b/core-web/libs/image-editor/src/lib/components/dot-image-editor-canvas/dot-image-editor-canvas.component.ts index 5b6ce098b74..4544c936d6a 100644 --- a/core-web/libs/image-editor/src/lib/components/dot-image-editor-canvas/dot-image-editor-canvas.component.ts +++ b/core-web/libs/image-editor/src/lib/components/dot-image-editor-canvas/dot-image-editor-canvas.component.ts @@ -515,7 +515,7 @@ export class DotImageEditorCanvasComponent { }; } - /** Lazily attaches a single ResizeObserver to the displayed image element. */ + /** Lazily attaches a single ResizeObserver to the displayed image and its stage. */ #observeDisplayImg(): void { const img = this.$displayImg()?.nativeElement; @@ -525,6 +525,19 @@ export class DotImageEditorCanvasComponent { this.#resizeObserver = new ResizeObserver(() => this.#measureImageRect()); this.#resizeObserver.observe(img); + + // Also observe the stage. Toggling full-screen resizes the dialog, which + // re-centres the image within the stage WITHOUT changing the image's own size + // — a ResizeObserver on the image alone never reports that, leaving + // imageRect.x/y (img.offsetLeft/Top) stale and the focal/crop overlays + // mispositioned (notably on the full-screen -> windowed transition). The stage + // (the image's offsetParent) always resizes with the dialog, so observing it + // re-measures the image's offset box and keeps the overlays aligned. + const stage = this.$stage()?.nativeElement; + + if (stage) { + this.#resizeObserver.observe(stage); + } } /** diff --git a/core-web/libs/image-editor/src/lib/components/dot-image-editor-focal-overlay/dot-image-editor-focal-overlay.component.scss b/core-web/libs/image-editor/src/lib/components/dot-image-editor-focal-overlay/dot-image-editor-focal-overlay.component.scss index 1345ef265ec..3aa1b8a8b98 100644 --- a/core-web/libs/image-editor/src/lib/components/dot-image-editor-focal-overlay/dot-image-editor-focal-overlay.component.scss +++ b/core-web/libs/image-editor/src/lib/components/dot-image-editor-focal-overlay/dot-image-editor-focal-overlay.component.scss @@ -38,11 +38,13 @@ width: 6px; height: 6px; transform: translate(-50%, -50%); - background-color: var(--p-primary-color, #426bf0); + // Red target dot: high-contrast against arbitrary image content and the + // conventional colour for a focal-point marker (distinct from the UI primary). + background-color: var(--p-red-500, #ef4444); border-radius: 9999px; } &:focus-visible { - border-color: var(--p-primary-color, #426bf0); + border-color: var(--p-red-500, #ef4444); } } From e02278959b1a6efa73da06b2ad9c8c55cff2dce3 Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Mon, 29 Jun 2026 15:41:04 -0400 Subject: [PATCH 06/18] chore(image-editor): address local code-review findings - DefaultTransformStrategy: log (debug) when reading the focal point custom-meta fails instead of silently swallowing it in the Try. - binary-field-utils.spec: split the focal-point sentinel test so the (0,0) case and the malformed/single-value case are asserted separately (the latter hits the NaN branch, not the (0,0) sentinel). - onEditImage: document why focalPoint is read via a narrow cast (it is exposed at runtime but not declared on the shared DotFileMetadata model). --- .../dot-edit-content-binary-field.component.ts | 3 +++ .../utils/binary-field-utils.spec.ts | 9 +++++---- .../transform/strategy/DefaultTransformStrategy.java | 5 ++++- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/dot-edit-content-binary-field.component.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/dot-edit-content-binary-field.component.ts index e9d0c137472..0cc7e0eb32e 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/dot-edit-content-binary-field.component.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/dot-edit-content-binary-field.component.ts @@ -436,6 +436,9 @@ export class DotEditContentBinaryFieldComponent // Seed the editor with the asset's stored focal point (exposed on the binary // field metadata as a `"x,y"` string) so reopening restores the marker instead // of resetting it to centre. + // `focalPoint` is exposed on the binary metadata at runtime (DefaultTransformStrategy) + // but not declared on DotFileMetadata; read it through a narrow cast rather than + // extending the shared model (which would pull the lib into affected-lint). const focalPoint = parseFocalPoint( (metadata as { focalPoint?: string } | null)?.focalPoint ); diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/utils/binary-field-utils.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/utils/binary-field-utils.spec.ts index 3bc8fccf120..ce8d31f37ca 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/utils/binary-field-utils.spec.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/utils/binary-field-utils.spec.ts @@ -12,13 +12,14 @@ describe('binary-field-utils', () => { expect(parseFocalPoint('')).toBeUndefined(); }); - it('treats the backend "no focal point" sentinels (0,0) as undefined', () => { - // The backend uses "0.0"/"0,0"/(0,0) to mean "no focal point" -> editor opens centred. + it('treats the (0,0) "no focal point" sentinel as undefined', () => { + // The backend uses (0,0) to mean "no focal point" -> editor opens centred. expect(parseFocalPoint('0,0')).toBeUndefined(); - expect(parseFocalPoint('0.0')).toBeUndefined(); }); - it('returns undefined for a malformed value', () => { + it('returns undefined for a malformed or single-value string', () => { + // "0.0" has no comma -> one token -> the y axis parses to NaN. + expect(parseFocalPoint('0.0')).toBeUndefined(); expect(parseFocalPoint('not-a-point')).toBeUndefined(); expect(parseFocalPoint('abc,xyz')).toBeUndefined(); }); diff --git a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/transform/strategy/DefaultTransformStrategy.java b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/transform/strategy/DefaultTransformStrategy.java index 4d6b4be0364..616e6a6772d 100644 --- a/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/transform/strategy/DefaultTransformStrategy.java +++ b/dotCMS/src/main/java/com/dotmarketing/portlets/contentlet/transform/strategy/DefaultTransformStrategy.java @@ -270,7 +270,10 @@ private void addBinaries(final Contentlet contentlet, final Map // it explicitly so the image editor can re-seed its marker when reopened. final String focalPoint = Try.of(() -> metadata.getCustomMeta().getOrDefault(FocalPointAPI.FOCAL_POINT, "0.0").toString() - ).getOrElse("0.0"); + ).onFailure(e -> Logger.debug(this, + "Unable to read focal point for field '" + velocityVarName + "': " + + e.getMessage())) + .getOrElse("0.0"); metaMap.put(FocalPointAPI.FOCAL_POINT, focalPoint); map.put(velocityVarName + "MetaData", metaMap); putBinaryLinks(velocityVarName, metadata.getName(), contentlet, map); From 8ac580b857547e49044e357e87a7495576f3898a Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Mon, 29 Jun 2026 16:15:14 -0400 Subject: [PATCH 07/18] fix(image-editor): persist focal point on image-tool save temp Write the focal point directly onto the generated temp file in BinaryExporterServlet after copyMetadata, reading it from the request params. This guarantees the focal survives even when the image exporter serves the rendition from cache and skips the FocalPoint filter (the side effect that otherwise persists it). putCustomMetadataAttributes merges, so the metadata copied from the source is preserved. Add FocalPointAPITest coverage for the parse-from-params -> write-to-temp -> read-back composition the servlet performs on Save. --- .../servlets/BinaryExporterServlet.java | 12 ++++++ .../image/focalpoint/FocalPointAPITest.java | 42 ++++++++++++++++++- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/dotCMS/src/main/java/com/dotmarketing/servlets/BinaryExporterServlet.java b/dotCMS/src/main/java/com/dotmarketing/servlets/BinaryExporterServlet.java index 9e0ae611951..8b871090d82 100644 --- a/dotCMS/src/main/java/com/dotmarketing/servlets/BinaryExporterServlet.java +++ b/dotCMS/src/main/java/com/dotmarketing/servlets/BinaryExporterServlet.java @@ -51,6 +51,7 @@ import com.dotmarketing.beans.Identifier; import com.dotmarketing.business.APILocator; import com.dotmarketing.business.web.WebAPILocator; +import com.dotmarketing.image.focalpoint.FocalPointAPI; import com.dotmarketing.db.DbConnectionFactory; import com.dotmarketing.exception.DotDataException; import com.dotmarketing.exception.DotRuntimeException; @@ -108,6 +109,7 @@ public class BinaryExporterServlet extends HttpServlet { private final ContentletAPI contentAPI = APILocator.getContentletAPI(); private final TempFileAPI tempFileAPI = APILocator.getTempFileAPI(); private final FileMetadataAPI fileMetadataAPI = APILocator.getFileMetadataAPI(); + private final FocalPointAPI focalPointAPI = APILocator.getFocalPointAPI(); private final WebResource webResource = new WebResource(); Map exportersByPathMapping; @@ -424,6 +426,16 @@ public void doGet(HttpServletRequest req, HttpServletResponse resp) throws Servl temp = new DotTempFile(temp.id, temp.file); } copyMetadata(uuid, fieldVarName, temp); + // The focal point is authoritative in the request URL. Write it straight onto the saved + // temp AFTER copyMetadata, so it persists even when the image exporter served the rendition + // from cache and never ran FocalPointImageFilter (the side effect that otherwise persists + // the focal). No-op when the request carries no fp (plain saves / non-focal images); + // putCustomMetadataAttributes merges, so only the focalPoint key is set and the metadata + // copied above is preserved. + final String savedTempId = temp.id; + final String focalFieldVar = fieldVarName; + focalPointAPI.parseFocalPointFromParams(params) + .ifPresent(focalPoint -> focalPointAPI.writeFocalPoint(savedTempId, focalFieldVar, focalPoint)); resp.getWriter().println(DotObjectMapperProvider.getInstance().getDefaultObjectMapper().writeValueAsString(temp)); resp.getWriter().close(); resp.flushBuffer(); diff --git a/dotcms-integration/src/test/java/com/dotmarketing/image/focalpoint/FocalPointAPITest.java b/dotcms-integration/src/test/java/com/dotmarketing/image/focalpoint/FocalPointAPITest.java index 4a102ce0782..dc934b25b1b 100644 --- a/dotcms-integration/src/test/java/com/dotmarketing/image/focalpoint/FocalPointAPITest.java +++ b/dotcms-integration/src/test/java/com/dotmarketing/image/focalpoint/FocalPointAPITest.java @@ -177,6 +177,44 @@ public void Test_Write_Focal_Point_From_Temp_Asset_Id() throws Exception { assertFalse(focalPointAPI.readFocalPoint(invalidAssetID, "fileAsset").isPresent()); } - - + + /** + * Method to test: the focal-point persistence wired into the image-tool save flow. On Save, + * {@link com.dotmarketing.servlets.BinaryExporterServlet} reads the focal point from the + * request params ({@link FocalPointAPI#parseFocalPointFromParams(Map)}) and writes it directly + * onto the generated temp file ({@link FocalPointAPI#writeFocalPoint(String, String, FocalPoint)}). + *

+ * Test scenario: parse a focal point from a request param map, write it onto a freshly created + * temp file, and read it back — mirroring exactly what the servlet does on Save. The direct + * write is what makes the focal survive even when the image exporter serves the rendition from + * cache and never runs the FocalPoint filter (the side effect that otherwise persists it). + *

+ * Expected: the focal point is persisted on the saved temp and reads back equal. + */ + @Test + public void Test_Persist_Focal_Point_On_Temp_From_Save_Params() throws Exception { + final String fieldVar = "fileAsset"; + final FocalPoint expected = new FocalPoint(0.2f, 0.3f); + + // 1. The servlet pulls the focal point out of the request params (the /fp/x,y URL segment). + final Map params = ImmutableMap.of("fp", new String[] {"0.2,0.3"}); + final Optional parsed = focalPointAPI.parseFocalPointFromParams(params); + assertTrue("Focal point parsed from save params", parsed.isPresent()); + + // 2. A temp file is generated for the edited image. + final HttpServletRequest request = mockHttpServletRequest(); + final DotTempFile dotTempFile = APILocator.getTempFileAPI().createEmptyTempFile("temp", request); + assertTrue(dotTempFile.file.createNewFile()); + assertTrue(dotTempFile.file.setLastModified(System.currentTimeMillis())); + + // 3. The servlet writes the focal point straight onto that temp (independent of the filter). + focalPointAPI.writeFocalPoint(dotTempFile.id, fieldVar, parsed.get()); + + // 4. Reopening the editor reads the focal back from the temp's metadata. + final Optional persisted = focalPointAPI.readFocalPoint(dotTempFile.id, fieldVar); + assertTrue("Focal point persisted on the saved temp", persisted.isPresent()); + assertEquals(expected, persisted.get()); + } + + } From 35d824092a315fcea9b4a25b19dcf352c07c3189 Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Mon, 29 Jun 2026 16:20:43 -0400 Subject: [PATCH 08/18] docs(edit-content): correct stale comment on binary-field save-temp metadata guard The comment claimed image-editor save temps come back with metadata:null. The servlet now re-wraps the temp to regenerate metadata from the populated file, so it is normally non-null; the guard remains defensive for the unreadable-file case. --- .../store/binary-field.store.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/store/binary-field.store.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/store/binary-field.store.ts index d5892a63b26..3e2804802f8 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/store/binary-field.store.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/store/binary-field.store.ts @@ -212,9 +212,10 @@ export class DotBinaryFieldStore extends ComponentStore { private handleTempFile(tempFile: DotCMSTempFile): Observable { const { referenceUrl, metadata } = tempFile; - // Temp files minted by the image editor save (`_imageToolSaveFile`) come back with - // `metadata: null` (the servlet builds the metadata from the empty temp before copying - // the filtered bytes), so guard the destructure — `= false` only defaults `undefined`. + // Defensive: the servlet re-wraps the image-editor save temp (`_imageToolSaveFile`) to + // regenerate metadata from the populated file, so it is normally non-null — but it can + // still be null if the file is unreadable or metadata generation fails. Guard the + // destructure (`= false` only defaults `undefined`, not `null`). const { editableAsText = false } = metadata ?? {}; const obs$ = editableAsText ? this.getFileContent(referenceUrl) : of(''); From f4a24f6fb079e02e579a73d1ab648e8faa415c49 Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Mon, 29 Jun 2026 16:39:07 -0400 Subject: [PATCH 09/18] fix(image-editor): URL-encode binaryFieldId in save URL Wrap the field variable in encodeURIComponent when building the image-tool save URL so a stray &/=/? in the field name cannot break out of the value and inject query parameters. Field variables are identifier-constrained today, so this is defensive. Adds a builder spec asserting the encoding. --- .../src/lib/utils/image-filter-url.builder.spec.ts | 7 +++++++ .../image-editor/src/lib/utils/image-filter-url.builder.ts | 5 ++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.spec.ts b/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.spec.ts index a3691677535..596404ce317 100644 --- a/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.spec.ts +++ b/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.spec.ts @@ -399,5 +399,12 @@ describe('image-filter-url.builder', () => { const url = buildSaveUrl(baseContext, [], center, center, 'myImage'); expect(url).toContain('binaryFieldId=myImage'); }); + + it('URL-encodes the binaryFieldId so special characters cannot inject query params', () => { + const url = buildSaveUrl(baseContext, [], center, center, 'a&b=c'); + // The raw `&`/`=` must be percent-encoded, not leak into the query string. + expect(url).toContain('binaryFieldId=a%26b%3Dc'); + expect(url).not.toContain('binaryFieldId=a&b=c'); + }); }); }); diff --git a/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.ts b/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.ts index 5edd725887f..f74c68018ed 100644 --- a/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.ts +++ b/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.ts @@ -241,7 +241,10 @@ export function buildSaveUrl( url = cleanUrl(url); url += url.includes('?') ? '&' : '?'; - url += `binaryFieldId=${binaryFieldId}&_imageToolSaveFile=true`; + // Encode the field variable defensively: dotCMS field variables are constrained to + // identifier-safe characters today, but encoding guarantees a stray `&`/`=`/`?` could + // never break out of the value and inject extra query parameters. + url += `binaryFieldId=${encodeURIComponent(binaryFieldId)}&_imageToolSaveFile=true`; if (focalApplied) { url += '&overwrite=true'; From af17608725e4c9a4bf43e56586d0cddc058e2649 Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Mon, 29 Jun 2026 17:09:45 -0400 Subject: [PATCH 10/18] feat(image-editor): GA the new image editor in the new Edit Content (remove FF) The new image editor is used whenever the AngularImageEditorLauncher is provided (by the new Edit Content shell); the legacy Dojo editor is reached only when no launcher is present (outside the new Edit Content). The old Edit Content is untouched, so old edit contentlet = old image editor and new = new. Removes the launcher's feature-flag read (isAvailable() is now always true) and the backend FEATURE_FLAG_NEW_IMAGE_EDITOR plumbing: the FeatureFlagName constant, the dotmarketing-config property, and both ConfigurationResource exposures. --- ...dit-content-binary-field.component.spec.ts | 2 +- ...dot-edit-content-binary-field.component.ts | 6 ++--- .../angular-image-editor.launcher.spec.ts | 24 ++++--------------- .../angular-image-editor.launcher.ts | 21 +++++----------- .../dotcms/featureflag/FeatureFlagName.java | 8 ------- .../api/v1/system/ConfigurationResource.java | 2 -- .../resources/dotmarketing-config.properties | 4 ---- 7 files changed, 14 insertions(+), 53 deletions(-) diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/dot-edit-content-binary-field.component.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/dot-edit-content-binary-field.component.spec.ts index 6a292f94580..2c614cc707a 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/dot-edit-content-binary-field.component.spec.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/dot-edit-content-binary-field.component.spec.ts @@ -398,7 +398,7 @@ describe('DotEditContentBinaryFieldComponent', () => { expect(spyTempFile).not.toHaveBeenCalled(); }); - it('should fall back to the legacy Dojo editor when the new editor is disabled', () => { + it('should fall back to the legacy Dojo editor when no launcher is available', () => { imageEditorLauncherMock.isAvailable.mockReturnValue(false); const spyLegacy = jest .spyOn(DotBinaryFieldEditImageService.prototype, 'openImageEditor') diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/dot-edit-content-binary-field.component.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/dot-edit-content-binary-field.component.ts index 0cc7e0eb32e..e5872e38246 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/dot-edit-content-binary-field.component.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/dot-edit-content-binary-field.component.ts @@ -407,9 +407,9 @@ export class DotEditContentBinaryFieldComponent onEditImage() { const launcher = this.#imageEditorLauncher; - // The new Angular editor is gated by FEATURE_FLAG_NEW_IMAGE_EDITOR (via the - // launcher's `isAvailable()`). When it's off — or no launcher is provided in - // this context — fall back to the legacy Dojo image editor. + // The new Angular editor is used whenever a launcher is provided (the new Edit + // Content shell provides it). When no launcher is present — e.g. the field renders + // outside the new Edit Content — fall back to the legacy Dojo image editor. if (!launcher?.isAvailable()) { this.#dotBinaryFieldEditImageService.openImageEditor({ inode: this.contentlet?.inode, diff --git a/core-web/libs/edit-content/src/lib/fields/shared/image-editor-launcher/angular-image-editor.launcher.spec.ts b/core-web/libs/edit-content/src/lib/fields/shared/image-editor-launcher/angular-image-editor.launcher.spec.ts index 101b290129b..12cb3e239fb 100644 --- a/core-web/libs/edit-content/src/lib/fields/shared/image-editor-launcher/angular-image-editor.launcher.spec.ts +++ b/core-web/libs/edit-content/src/lib/fields/shared/image-editor-launcher/angular-image-editor.launcher.spec.ts @@ -1,11 +1,10 @@ import { expect } from '@jest/globals'; -import { createServiceFactory, mockProvider, SpectatorService } from '@ngneat/spectator/jest'; -import { BehaviorSubject, Subject } from 'rxjs'; +import { createServiceFactory, SpectatorService } from '@ngneat/spectator/jest'; +import { Subject } from 'rxjs'; import { DialogService } from 'primeng/dynamicdialog'; -import { DotPropertiesService } from '@dotcms/data-access'; -import { DotCMSTempFile, FeaturedFlags } from '@dotcms/dotcms-models'; +import { DotCMSTempFile } from '@dotcms/dotcms-models'; import { DotImageEditorComponent, ImageEditorOpenParams } from '@dotcms/image-editor'; import { AngularImageEditorLauncher } from './angular-image-editor.launcher'; @@ -13,10 +12,6 @@ import { AngularImageEditorLauncher } from './angular-image-editor.launcher'; describe('AngularImageEditorLauncher', () => { let spectator: SpectatorService; let onClose: Subject; - // Drives the new-image-editor flag; `next()` flows through `toSignal` so the same - // service instance reflects on/off without re-creating it. - const featureFlag$ = new BehaviorSubject(true); - const getFeatureFlag = jest.fn(() => featureFlag$); const params: ImageEditorOpenParams = { inode: 'inode-1', @@ -26,28 +21,17 @@ describe('AngularImageEditorLauncher', () => { const createService = createServiceFactory({ service: AngularImageEditorLauncher, - providers: [mockProvider(DotPropertiesService, { getFeatureFlag })], mocks: [DialogService] }); beforeEach(() => { - // Default the flag ON so the open() tests below run the Angular path; the - // gating itself is covered by the dedicated tests. - featureFlag$.next(true); onClose = new Subject(); spectator = createService(); spectator.inject(DialogService).open.mockReturnValue({ onClose }); }); - it('should be available when FEATURE_FLAG_NEW_IMAGE_EDITOR is on', () => { + it('should always be available (the new editor is the editor for the new Edit Content)', () => { expect(spectator.service.isAvailable()).toBe(true); - expect(getFeatureFlag).toHaveBeenCalledWith(FeaturedFlags.FEATURE_FLAG_NEW_IMAGE_EDITOR); - }); - - it('should NOT be available when the feature flag is off', () => { - featureFlag$.next(false); - - expect(spectator.service.isAvailable()).toBe(false); }); it('should open the DotImageEditorComponent with a headerless, closable dialog that owns Esc', () => { diff --git a/core-web/libs/edit-content/src/lib/fields/shared/image-editor-launcher/angular-image-editor.launcher.ts b/core-web/libs/edit-content/src/lib/fields/shared/image-editor-launcher/angular-image-editor.launcher.ts index c6720f2146b..95b9fa29da0 100644 --- a/core-web/libs/edit-content/src/lib/fields/shared/image-editor-launcher/angular-image-editor.launcher.ts +++ b/core-web/libs/edit-content/src/lib/fields/shared/image-editor-launcher/angular-image-editor.launcher.ts @@ -1,12 +1,10 @@ import { Observable, map, take } from 'rxjs'; import { inject, Injectable } from '@angular/core'; -import { toSignal } from '@angular/core/rxjs-interop'; import { DialogService } from 'primeng/dynamicdialog'; -import { DotPropertiesService } from '@dotcms/data-access'; -import { DotCMSTempFile, FeaturedFlags } from '@dotcms/dotcms-models'; +import { DotCMSTempFile } from '@dotcms/dotcms-models'; import { DotImageEditorComponent, DotImageEditorLauncher, @@ -16,24 +14,17 @@ import { /** * Launches the Angular `@dotcms/image-editor` modal through PrimeNG's `DialogService`. * - * Gated behind the {@link FeaturedFlags.FEATURE_FLAG_NEW_IMAGE_EDITOR} feature flag: - * `isAvailable()` resolves to the server-configured value, which ships as `false` - * for rollback safety (mirroring the other new-editor flags), so the binary field - * falls back to the legacy Dojo editor until an admin enables it. + * Provided by the new Edit Content shell, so the new image editor is used whenever this + * launcher is injected. The binary field falls back to the legacy Dojo editor only when + * no launcher is present (i.e. the field renders outside the new Edit Content). */ @Injectable() export class AngularImageEditorLauncher implements DotImageEditorLauncher { readonly #dialogService = inject(DialogService); - readonly #propertiesService = inject(DotPropertiesService); - - /** Resolved value of the new-image-editor feature flag (off until the server replies). */ - readonly #enabled = toSignal( - this.#propertiesService.getFeatureFlag(FeaturedFlags.FEATURE_FLAG_NEW_IMAGE_EDITOR), - { initialValue: false } - ); + /** Always available: the new image editor is the editor for the new Edit Content. */ isAvailable(): boolean { - return this.#enabled(); + return true; } /** diff --git a/dotCMS/src/main/java/com/dotcms/featureflag/FeatureFlagName.java b/dotCMS/src/main/java/com/dotcms/featureflag/FeatureFlagName.java index 24260456bfb..449302b4036 100644 --- a/dotCMS/src/main/java/com/dotcms/featureflag/FeatureFlagName.java +++ b/dotCMS/src/main/java/com/dotcms/featureflag/FeatureFlagName.java @@ -75,14 +75,6 @@ public interface FeatureFlagName { String FEATURE_FLAG_LOCALE_SELECTOR_V2 = "FEATURE_FLAG_LOCALE_SELECTOR_V2"; - /** - * Enables the new Angular image editor (@dotcms/image-editor) in the Edit Content v2 - * binary field. Off by default; when disabled the binary field falls back to the - * legacy Dojo image editor. - * Frontend equivalent: {@code FeaturedFlags.FEATURE_FLAG_NEW_IMAGE_EDITOR}. - */ - String FEATURE_FLAG_NEW_IMAGE_EDITOR = "FEATURE_FLAG_NEW_IMAGE_EDITOR"; - /** * libvips image-engine toggle (off by default; the legacy Java2D engine is used * otherwise). The new image editor reads this through the configuration endpoint diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/system/ConfigurationResource.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/system/ConfigurationResource.java index b6a0c1457fb..ef5af4f764f 100644 --- a/dotCMS/src/main/java/com/dotcms/rest/api/v1/system/ConfigurationResource.java +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/system/ConfigurationResource.java @@ -93,7 +93,6 @@ public class ConfigurationResource implements Serializable { FeatureFlagName.FEATURE_FLAG_NEW_BLOCK_EDITOR, FeatureFlagName.FEATURE_FLAG_CONTENT_EDITOR2_ENABLED, FeatureFlagName.FEATURE_FLAG_LOCALE_SELECTOR_V2, - FeatureFlagName.FEATURE_FLAG_NEW_IMAGE_EDITOR, // libvips engine toggle: the new image editor reads it to gate AVIF output. FeatureFlagName.IMAGE_API_USE_LIBVIPS); @@ -112,7 +111,6 @@ public class ConfigurationResource implements Serializable { REPORT_ISSUE_INCLUDE_USER_PII, FeatureFlagName.FEATURE_FLAG_REPORT_ISSUE_ENABLED, FeatureFlagName.FEATURE_FLAG_LOCALE_SELECTOR_V2, - FeatureFlagName.FEATURE_FLAG_NEW_IMAGE_EDITOR, // libvips engine toggle: the new image editor reads it to gate AVIF output. FeatureFlagName.IMAGE_API_USE_LIBVIPS })); diff --git a/dotCMS/src/main/resources/dotmarketing-config.properties b/dotCMS/src/main/resources/dotmarketing-config.properties index 90aa703758d..741b88578cc 100644 --- a/dotCMS/src/main/resources/dotmarketing-config.properties +++ b/dotCMS/src/main/resources/dotmarketing-config.properties @@ -873,10 +873,6 @@ FEATURE_FLAG_NEW_BLOCK_EDITOR=false ## Enhanced locale selector v2 in the edit-content sidebar FEATURE_FLAG_LOCALE_SELECTOR_V2=true -## New Angular image editor in the Edit Content v2 binary field (rollback safety: -## the legacy Dojo image editor opens by default until this is explicitly enabled) -FEATURE_FLAG_NEW_IMAGE_EDITOR=false - ## libvips image engine toggle. Off by default (legacy Java2D engine). The new image ## editor reads this (via the configuration endpoint) to gate the libvips-only AVIF ## output format. Declared here so the endpoint returns an explicit boolean instead From 2a2652ce1ae70408c1e494a46e4bbdde4eafe3c6 Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Mon, 29 Jun 2026 17:09:58 -0400 Subject: [PATCH 11/18] chore(dotcms-models): drop FEATURE_FLAG_NEW_IMAGE_EDITOR enum + fix pre-existing lint Removes the now-unused FeaturedFlags.FEATURE_FLAG_NEW_IMAGE_EDITOR enum member. Editing dotcms-models pulls it into nx affected:lint, which fails on 4 pre-existing errors unrelated to this change; fixed so the gate stays green: - relative imports in dot-contentlets-events.model.ts and page-model-change-event.ts - DotHttpRequestOptions.body: any -> XMLHttpRequestBodyInit | null - DotCMSWorkflowInput.body: any -> Record --- .../dotcms-models/src/lib/dot-contentlets-events.model.ts | 4 +++- .../dotcms-models/src/lib/dot-http-request-options.model.ts | 2 +- .../libs/dotcms-models/src/lib/dot-workflow-action.model.ts | 2 +- .../libs/dotcms-models/src/lib/page-model-change-event.ts | 3 +-- core-web/libs/dotcms-models/src/lib/shared-models.ts | 3 +-- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/core-web/libs/dotcms-models/src/lib/dot-contentlets-events.model.ts b/core-web/libs/dotcms-models/src/lib/dot-contentlets-events.model.ts index 9396e6299f6..497ddf60f35 100644 --- a/core-web/libs/dotcms-models/src/lib/dot-contentlets-events.model.ts +++ b/core-web/libs/dotcms-models/src/lib/dot-contentlets-events.model.ts @@ -1,4 +1,6 @@ -import { DotCMSContentType, DotPageContainer, DotPageContent } from '@dotcms/dotcms-models'; +import { DotCMSContentType } from './dot-content-types.model'; +import { DotPageContainer } from './dot-page-container.model'; +import { DotPageContent } from './dot-page-content.model'; export interface DotContentletEvent> { name: string; diff --git a/core-web/libs/dotcms-models/src/lib/dot-http-request-options.model.ts b/core-web/libs/dotcms-models/src/lib/dot-http-request-options.model.ts index 462a51f37dd..3c5b3702fa1 100644 --- a/core-web/libs/dotcms-models/src/lib/dot-http-request-options.model.ts +++ b/core-web/libs/dotcms-models/src/lib/dot-http-request-options.model.ts @@ -1,5 +1,5 @@ export interface DotHttpRequestOptions { method: string; headers: { [key: string]: string }; - body: any; + body: XMLHttpRequestBodyInit | null; } diff --git a/core-web/libs/dotcms-models/src/lib/dot-workflow-action.model.ts b/core-web/libs/dotcms-models/src/lib/dot-workflow-action.model.ts index 263457f288a..03a624ecb2d 100644 --- a/core-web/libs/dotcms-models/src/lib/dot-workflow-action.model.ts +++ b/core-web/libs/dotcms-models/src/lib/dot-workflow-action.model.ts @@ -70,7 +70,7 @@ export interface DotCMSSystemAction { export interface DotCMSWorkflowInput { id: string; - body: any; + body: Record; } export interface DotCMSContentletWorkflowActions { diff --git a/core-web/libs/dotcms-models/src/lib/page-model-change-event.ts b/core-web/libs/dotcms-models/src/lib/page-model-change-event.ts index a5edb3d90b5..6ead8e412ea 100644 --- a/core-web/libs/dotcms-models/src/lib/page-model-change-event.ts +++ b/core-web/libs/dotcms-models/src/lib/page-model-change-event.ts @@ -1,5 +1,4 @@ -import { DotPageContainer } from '@dotcms/dotcms-models'; - +import { DotPageContainer } from './dot-page-container.model'; import { PageModelChangeEventType } from './page-model-change-event.type'; export interface PageModelChangeEvent { diff --git a/core-web/libs/dotcms-models/src/lib/shared-models.ts b/core-web/libs/dotcms-models/src/lib/shared-models.ts index fe86425a80e..2da10347ece 100644 --- a/core-web/libs/dotcms-models/src/lib/shared-models.ts +++ b/core-web/libs/dotcms-models/src/lib/shared-models.ts @@ -36,8 +36,7 @@ export const enum FeaturedFlags { FEATURE_FLAG_UVE_LEGACY_SCRIPT_INJECTION = 'FEATURE_FLAG_UVE_LEGACY_SCRIPT_INJECTION', FEATURE_FLAG_NEW_BLOCK_EDITOR = 'FEATURE_FLAG_NEW_BLOCK_EDITOR', FEATURE_FLAG_REPORT_ISSUE_ENABLED = 'FEATURE_FLAG_REPORT_ISSUE_ENABLED', - FEATURE_FLAG_LOCALE_SELECTOR_V2 = 'FEATURE_FLAG_LOCALE_SELECTOR_V2', - FEATURE_FLAG_NEW_IMAGE_EDITOR = 'FEATURE_FLAG_NEW_IMAGE_EDITOR' + FEATURE_FLAG_LOCALE_SELECTOR_V2 = 'FEATURE_FLAG_LOCALE_SELECTOR_V2' } export const enum DotConfigurationVariables { From a6c002f8833249826e35757147620bdd37cc89f4 Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Tue, 30 Jun 2026 10:43:58 -0400 Subject: [PATCH 12/18] feat(image-editor): port focal-point seeding to the unified file-field #36172 unified the field components: the binary-field component was removed and image editing now lives in DotFileFieldComponent, which already wires the Angular launcher (gated only by the launcher's isAvailable(), now always true) and applies the edited temp via the store. Re-applies the one piece that did not survive the unify refactor: - onEditImage seeds the editor with the asset's stored focal point so reopening a saved binary image restores the marker instead of resetting it to centre. - Re-adds parseFocalPoint (lost with the deleted binary-field utils) as a file-field util, with its spec. - Drops the orphaned binary-field-utils.spec left behind by the merge. - Refreshes the stale FEATURE_FLAG_NEW_IMAGE_EDITOR comments (flag was removed). --- .../utils/binary-field-utils.spec.ts | 49 ------------------- .../dot-file-field.component.ts | 25 +++++++--- .../utils/focal-point.util.spec.ts | 25 ++++++++++ .../utils/focal-point.util.ts | 32 ++++++++++++ 4 files changed, 74 insertions(+), 57 deletions(-) delete mode 100644 core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/utils/binary-field-utils.spec.ts create mode 100644 core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/utils/focal-point.util.spec.ts create mode 100644 core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/utils/focal-point.util.ts diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/utils/binary-field-utils.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/utils/binary-field-utils.spec.ts deleted file mode 100644 index ce8d31f37ca..00000000000 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-binary-field/utils/binary-field-utils.spec.ts +++ /dev/null @@ -1,49 +0,0 @@ -import { getFileMetadata, parseFocalPoint } from './binary-field-utils'; - -describe('binary-field-utils', () => { - describe('parseFocalPoint', () => { - it('parses a "x,y" focal point string into a point', () => { - expect(parseFocalPoint('0.88,0.31')).toEqual({ x: 0.88, y: 0.31 }); - }); - - it('returns undefined for an unset/empty value', () => { - expect(parseFocalPoint(undefined)).toBeUndefined(); - expect(parseFocalPoint(null)).toBeUndefined(); - expect(parseFocalPoint('')).toBeUndefined(); - }); - - it('treats the (0,0) "no focal point" sentinel as undefined', () => { - // The backend uses (0,0) to mean "no focal point" -> editor opens centred. - expect(parseFocalPoint('0,0')).toBeUndefined(); - }); - - it('returns undefined for a malformed or single-value string', () => { - // "0.0" has no comma -> one token -> the y axis parses to NaN. - expect(parseFocalPoint('0.0')).toBeUndefined(); - expect(parseFocalPoint('not-a-point')).toBeUndefined(); - expect(parseFocalPoint('abc,xyz')).toBeUndefined(); - }); - }); - - describe('getFileMetadata', () => { - it('returns an empty object when the contentlet is null', () => { - // The binary-field preview falls back here for image-editor temp files - // (which carry no contentlet and `metadata: null`), so it must not throw. - expect(getFileMetadata(null as never)).toEqual({}); - }); - - it('returns the contentlet metaData when present', () => { - const metaData = { isImage: true, contentType: 'image/png' }; - - expect(getFileMetadata({ metaData } as never)).toBe(metaData); - }); - - it('falls back to the {fieldVariable}MetaData entry', () => { - const assetMetaData = { isImage: true }; - - expect(getFileMetadata({ fieldVariable: 'asset', assetMetaData } as never)).toBe( - assetMetaData - ); - }); - }); -}); diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts index 995b48b7dd7..55d21889a7f 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts @@ -48,6 +48,7 @@ import { } from './../../services/image-editor'; import { DotFileFieldUploadService } from './../../services/upload-file/upload-file.service'; import { FileFieldStore } from './../../store/file-field.store'; +import { parseFocalPoint } from './../../utils/focal-point.util'; import { getUiMessage } from './../../utils/messages'; import { DotFileFieldPreviewComponent } from './../dot-file-field-preview/dot-file-field-preview.component'; import { DotFileFieldUiMessageComponent } from './../dot-file-field-ui-message/dot-file-field-ui-message.component'; @@ -104,10 +105,10 @@ export class DotFileFieldComponent readonly #legacyDojoImageEditorLauncher = inject(LegacyDojoImageEditorLauncher); /** * New Angular image editor launcher. Provided by the Angular edit-content shell - * ({@link EditContentShellComponent}) and gated behind the - * `FEATURE_FLAG_NEW_IMAGE_EDITOR` flag via its `isAvailable()`. Injected as - * `{ optional: true }`: when absent (e.g. the legacy web-component host) or its - * flag is off, `onEditImage()` falls back to the legacy launchers. + * ({@link EditContentShellComponent}); its `isAvailable()` is always true now that + * the new editor is GA in the new Edit Content. Injected as `{ optional: true }`: + * when absent (e.g. the legacy web-component host), `onEditImage()` falls back to + * the legacy launchers. */ readonly #imageEditorLauncher = inject(IMAGE_EDITOR_LAUNCHER, { optional: true }); /** @@ -408,13 +409,20 @@ export class DotFileFieldComponent : variable; // Prefer the new Angular image editor when its launcher is provided (Angular - // edit-content shell) and its feature flag is on. Otherwise fall back to the - // legacy editor: Dojo DOM events for the web-component bridge, dialog iframe - // elsewhere. + // edit-content shell). Otherwise fall back to the legacy editor: Dojo DOM events + // for the web-component bridge, dialog iframe elsewhere. const newLauncher = this.#imageEditorLauncher; if (newLauncher?.isAvailable()) { const metadata = this.#currentMetadata(); + // Seed the editor with the asset's stored focal point (exposed on the binary + // metadata as an "x,y" string by DefaultTransformStrategy) so reopening + // restores the marker instead of resetting it to centre. `focalPoint` is + // present at runtime but not declared on DotFileMetadata, so read it through + // a narrow cast rather than widening the shared model. + const focalPoint = parseFocalPoint( + (metadata as { focalPoint?: string } | null)?.focalPoint + ); this.#applyEditedImage( newLauncher.open({ @@ -424,7 +432,8 @@ export class DotFileFieldComponent fieldName: editorVariable, byInode: !!inode, fileName: this.$currentFileName() || undefined, - mimeType: metadata?.contentType + mimeType: metadata?.contentType, + focalPoint }) ); diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/utils/focal-point.util.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/utils/focal-point.util.spec.ts new file mode 100644 index 00000000000..111bc6c2bba --- /dev/null +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/utils/focal-point.util.spec.ts @@ -0,0 +1,25 @@ +import { parseFocalPoint } from './focal-point.util'; + +describe('parseFocalPoint', () => { + it('parses an "x,y" focal point string into a point', () => { + expect(parseFocalPoint('0.88,0.31')).toEqual({ x: 0.88, y: 0.31 }); + }); + + it('returns undefined for an unset/empty value', () => { + expect(parseFocalPoint(undefined)).toBeUndefined(); + expect(parseFocalPoint(null)).toBeUndefined(); + expect(parseFocalPoint('')).toBeUndefined(); + }); + + it('treats the (0,0) "no focal point" sentinel as undefined', () => { + // The backend uses (0,0) to mean "no focal point" -> editor opens centred. + expect(parseFocalPoint('0,0')).toBeUndefined(); + }); + + it('returns undefined for a malformed or single-value string', () => { + // "0.0" has no comma -> one token -> the y axis parses to NaN. + expect(parseFocalPoint('0.0')).toBeUndefined(); + expect(parseFocalPoint('not-a-point')).toBeUndefined(); + expect(parseFocalPoint('abc,xyz')).toBeUndefined(); + }); +}); diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/utils/focal-point.util.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/utils/focal-point.util.ts new file mode 100644 index 00000000000..a5b59d65e62 --- /dev/null +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/utils/focal-point.util.ts @@ -0,0 +1,32 @@ +import { NormalizedPoint } from '@dotcms/image-editor'; + +/** + * Parses a focal point stored as an `"x,y"` string (the backend exposes it on the + * binary field metadata as a custom `focalPoint` attribute) into a normalized 0..1 + * point for seeding the image editor. + * + * Returns `undefined` for an unset/empty value, the `(0,0)` "no focal point" sentinel, + * or a malformed/single-value string, so the editor opens centred instead of seeding a + * bogus marker. + * + * @param value - The raw `focalPoint` metadata string (e.g. `"0.88,0.31"`). + * @returns The parsed point, or `undefined` when there is no usable focal point. + */ +export function parseFocalPoint(value: string | null | undefined): NormalizedPoint | undefined { + if (!value) { + return undefined; + } + + const [x, y] = value.split(',').map(Number); + + if (!Number.isFinite(x) || !Number.isFinite(y)) { + return undefined; + } + + // (0,0) is the backend's "no focal point" sentinel -> open centred. + if (x === 0 && y === 0) { + return undefined; + } + + return { x, y }; +} From 3b15607ceb1d5363b860b9f73f2f254a3d00458c Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Tue, 30 Jun 2026 12:50:13 -0400 Subject: [PATCH 13/18] fix(image-editor): use exhaustMap for the save effect switchMap cancelled an in-flight save when saveRequested re-fired; exhaustMap ignores re-triggers while a save is in flight (correct save semantics, no lost request). The Save button is already disabled while saving, so this is defense-in-depth for non-UI triggers. --- .../src/lib/store/features/with-save.feature.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/core-web/libs/image-editor/src/lib/store/features/with-save.feature.ts b/core-web/libs/image-editor/src/lib/store/features/with-save.feature.ts index 362373d2fab..3f38521a9fc 100644 --- a/core-web/libs/image-editor/src/lib/store/features/with-save.feature.ts +++ b/core-web/libs/image-editor/src/lib/store/features/with-save.feature.ts @@ -4,7 +4,7 @@ import { Dispatcher, Events, on, withEventHandlers, withReducer } from '@ngrx/si import { inject, Signal } from '@angular/core'; -import { switchMap } from 'rxjs/operators'; +import { exhaustMap } from 'rxjs/operators'; import { AppliedFilter, ImageEditorState } from '../../models/image-editor.models'; import { DotImageEditorService } from '../../services/dot-image-editor.service'; @@ -44,9 +44,12 @@ export function withSave() { const service = inject(DotImageEditorService); return { - // Save the edited image whenever the user requests it. + // Save the edited image whenever the user requests it. `exhaustMap` (not + // switchMap) so a re-triggered save is ignored while one is in flight, rather + // than cancelling the in-flight request. The Save button is already disabled + // while saving, so this is defense-in-depth for non-UI triggers. save$: events.on(imageEditorLifecycleEvents.saveRequested).pipe( - switchMap(() => + exhaustMap(() => service .saveEditedImage( buildSaveUrl( From 5025a83f43c76ed88b6434e8e8abec07521a05a4 Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Tue, 30 Jun 2026 13:16:19 -0400 Subject: [PATCH 14/18] fix(sdk-ai): default generate-spec to corpsites-headless instance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit demo.dotcms.com returned 503 and broke the sdk-ai:generate-spec CI step (the no-env fallback fetched its OpenAPI spec from demo). Point the default at corpsites-headless.dotcms.cloud, which serves the same REST API contract — the filtered spec.json is byte-identical, so no regeneration diff. --- core-web/libs/sdk/ai/scripts/generate-spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core-web/libs/sdk/ai/scripts/generate-spec.ts b/core-web/libs/sdk/ai/scripts/generate-spec.ts index e48a31947d2..6692590b1e0 100644 --- a/core-web/libs/sdk/ai/scripts/generate-spec.ts +++ b/core-web/libs/sdk/ai/scripts/generate-spec.ts @@ -58,7 +58,7 @@ const EXCLUDED_PATTERNS = [ ]; const DEFAULT_SPEC_PATH = '/api/openapi.json'; -const DEFAULT_SPEC_URL = `https://demo.dotcms.com${DEFAULT_SPEC_PATH}`; +const DEFAULT_SPEC_URL = `https://corpsites-headless.dotcms.cloud${DEFAULT_SPEC_PATH}`; /** * Matches a path against a pattern. Pattern syntax: From 00659db917af57e3c186ebc4624a57f60aa9c6b8 Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Tue, 30 Jun 2026 14:10:05 -0400 Subject: [PATCH 15/18] fix(image-editor): seed focal point on in-session reopen after save MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The editor Save returns a DotCMSTempFile whose serialized metadata is basic (the servlet writes the focal to the temp's metadata storage, not the returned object), so reopening the editor in the same session read a focal-less temp and re-seeded the marker at centre — while a page refresh (which reads the contentlet metadata, where the backend exposes focalPoint) showed it correctly. Fold the current focal point into the returned temp's metadata in the save effect so the in-session reopen matches the post-refresh read. Adds focalPoint to DotFileMetadata (now that dotcms-models lint is clean) which also drops the prior narrow cast in onEditImage. --- .../src/lib/dot-file-metadata.model.ts | 5 ++++ .../dot-file-field.component.ts | 10 ++----- .../store/features/with-save.feature.spec.ts | 27 ++++++++++++++++++ .../lib/store/features/with-save.feature.ts | 28 +++++++++++++++---- 4 files changed, 57 insertions(+), 13 deletions(-) diff --git a/core-web/libs/dotcms-models/src/lib/dot-file-metadata.model.ts b/core-web/libs/dotcms-models/src/lib/dot-file-metadata.model.ts index f441f4ea3c8..0416232edfb 100644 --- a/core-web/libs/dotcms-models/src/lib/dot-file-metadata.model.ts +++ b/core-web/libs/dotcms-models/src/lib/dot-file-metadata.model.ts @@ -11,4 +11,9 @@ export interface DotFileMetadata { height?: number; width?: number; editableAsText?: boolean; + /** + * Focal point as an `"x,y"` string (normalized 0..1), exposed by the backend on image + * binary metadata and used to re-seed the image editor's focal marker on reopen. + */ + focalPoint?: string; } diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts index 55d21889a7f..9b1f531332e 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-file-field/components/dot-file-field/dot-file-field.component.ts @@ -416,13 +416,9 @@ export class DotFileFieldComponent if (newLauncher?.isAvailable()) { const metadata = this.#currentMetadata(); // Seed the editor with the asset's stored focal point (exposed on the binary - // metadata as an "x,y" string by DefaultTransformStrategy) so reopening - // restores the marker instead of resetting it to centre. `focalPoint` is - // present at runtime but not declared on DotFileMetadata, so read it through - // a narrow cast rather than widening the shared model. - const focalPoint = parseFocalPoint( - (metadata as { focalPoint?: string } | null)?.focalPoint - ); + // metadata as an "x,y" string by DefaultTransformStrategy) so reopening restores + // the marker instead of resetting it to centre. + const focalPoint = parseFocalPoint(metadata?.focalPoint); this.#applyEditedImage( newLauncher.open({ diff --git a/core-web/libs/image-editor/src/lib/store/features/with-save.feature.spec.ts b/core-web/libs/image-editor/src/lib/store/features/with-save.feature.spec.ts index 4e724c07d68..88f3b58d98c 100644 --- a/core-web/libs/image-editor/src/lib/store/features/with-save.feature.spec.ts +++ b/core-web/libs/image-editor/src/lib/store/features/with-save.feature.spec.ts @@ -106,6 +106,33 @@ describe('withSave', () => { expect(store.saveError()).toBeNull(); }); + it('folds the current focal point into the returned temp metadata', () => { + // The servlet returns a temp whose serialized metadata is basic (no focalPoint), + // so the feature injects the focal so an in-session reopen can re-seed the marker. + const metadata = { + contentType: 'image/png', + fileSize: 1024, + isImage: true, + length: 1024, + modDate: 0, + name: 'edited.png', + sha256: 'abc', + title: 'edited.png', + version: 1 + }; + service.saveEditedImage.mockReturnValue(of({ ...TEMP_FILE, metadata })); + const dispatchSpy = setup(); + + lifecycle.saveRequested(); + + expect(dispatchSpy).toHaveBeenCalledWith( + imageEditorLifecycleEvents.saveSucceeded({ + ...TEMP_FILE, + metadata: { ...metadata, focalPoint: '0.25,0.75' } + }) + ); + }); + it('builds the Save URL from the asset context, filter chain and focal point', () => { setup(); diff --git a/core-web/libs/image-editor/src/lib/store/features/with-save.feature.ts b/core-web/libs/image-editor/src/lib/store/features/with-save.feature.ts index 3f38521a9fc..c12fa101d96 100644 --- a/core-web/libs/image-editor/src/lib/store/features/with-save.feature.ts +++ b/core-web/libs/image-editor/src/lib/store/features/with-save.feature.ts @@ -49,13 +49,21 @@ export function withSave() { // than cancelling the in-flight request. The Save button is already disabled // while saving, so this is defense-in-depth for non-UI triggers. save$: events.on(imageEditorLifecycleEvents.saveRequested).pipe( - exhaustMap(() => - service + exhaustMap(() => { + // Capture the focal point at save time and fold it into the returned + // temp's metadata. The servlet writes the focal to the temp's metadata + // STORAGE, but the serialized DotCMSTempFile.metadata is basic (no + // focalPoint), so an in-session reopen would read the temp metadata and + // seed the marker at centre. Injecting it here mirrors the contentlet + // read-back used after a page refresh, keeping both paths consistent. + const focalPoint = store.focalPoint(); + + return service .saveEditedImage( buildSaveUrl( store.assetContext(), store.appliedFilters(), - store.focalPoint(), + focalPoint, store.seededFocalPoint(), store.assetContext().variable ) @@ -64,15 +72,23 @@ export function withSave() { tapResponse({ next: (tempFile) => dispatcher.dispatch( - imageEditorLifecycleEvents.saveSucceeded(tempFile) + imageEditorLifecycleEvents.saveSucceeded({ + ...tempFile, + metadata: tempFile.metadata + ? { + ...tempFile.metadata, + focalPoint: `${focalPoint.x},${focalPoint.y}` + } + : tempFile.metadata + }) ), error: (error) => dispatcher.dispatch( imageEditorLifecycleEvents.saveFailed(error) ) }) - ) - ) + ); + }) ) }; }) From 7f85a41b1495c589168d3310036f62b03cd86373 Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Tue, 30 Jun 2026 15:53:34 -0400 Subject: [PATCH 16/18] test(e2e): assert the new Angular image editor opens in the new Edit Content Removing FEATURE_FLAG_NEW_IMAGE_EDITOR made the new Edit Content always open the new Angular image editor, so the binary-field E2E that waited for the legacy Dojo iframe (legacy-image-editor-iframe) timed out. Assert the new editor's dialog (image-editor-root) instead. The legacy-editor describe block (legacy Edit Content, CONTENT_EDITOR2_ENABLED=false) is unchanged and still asserts the Dojo editor. --- .../binary-field-image-editor.spec.ts | 8 +++++--- .../binary-field/helpers/binary-field.ts | 15 ++++++--------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/binary-field/binary-field-image-editor.spec.ts b/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/binary-field/binary-field-image-editor.spec.ts index f43fb40d6e9..cae5890845f 100644 --- a/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/binary-field/binary-field-image-editor.spec.ts +++ b/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/binary-field/binary-field-image-editor.spec.ts @@ -56,9 +56,11 @@ test.describe('Binary field image editor — new editor', () => { } }); - // The unified Binary field shows "Edit image" for image files. The new - // editor opens the legacy image editor JSP inside a PrimeNG dialog iframe. - test('import image and Edit opens legacy image editor dialog @critical', async ({ page }) => { + // The unified Binary field shows "Edit image" for image files. In the new + // Edit Content the new Angular image editor opens (the FEATURE_FLAG_NEW_IMAGE_EDITOR + // gate was removed, so it is always used here; the legacy Dojo editor only runs in + // the legacy Edit Content — see the describe block below). + test('import image and Edit opens the new Angular image editor @critical', async ({ page }) => { const formPage = new NewEditContentFormPage(page); await formPage.goToNew(contentTypeVariable); diff --git a/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/binary-field/helpers/binary-field.ts b/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/binary-field/helpers/binary-field.ts index 6ee2601f43d..5a6f2e3c9ad 100644 --- a/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/binary-field/helpers/binary-field.ts +++ b/core-web/apps/dotcms-ui-e2e/src/tests/edit-content/fields/file-upload-fields/binary-field/helpers/binary-field.ts @@ -96,20 +96,17 @@ export class BinaryField extends FileField { } /** - * New editor: the legacy image editor JSP is embedded in a PrimeNG dialog - * iframe (`dot-legacy-image-editor-dialog`), not the Dojo `#dotImageDialog`. + * New editor: clicking "Edit image" opens the Angular image editor + * ({@link DotImageEditorComponent}, `data-testid="image-editor-root"`) inside a + * PrimeNG dialog. The flag that used to gate it (FEATURE_FLAG_NEW_IMAGE_EDITOR) was + * removed, so the new editor is always used in the new Edit Content — there is no + * legacy Dojo iframe here. */ async expectImageEditorOpen() { const dialog = this.page.getByRole('dialog'); await expect(dialog).toBeVisible({ timeout: 15000 }); - const iframe = dialog.getByTestId('legacy-image-editor-iframe'); - await expect(iframe).toBeVisible({ timeout: 30000 }); - - const editorFrame = this.page.frameLocator('[data-testid="legacy-image-editor-iframe"]'); - await expect(editorFrame.locator('#dotImageDialog, #imageToolIframe').first()).toBeVisible({ - timeout: 30000 - }); + await expect(dialog.getByTestId('image-editor-root')).toBeVisible({ timeout: 30000 }); } async openImageEditorInNewEditor() { From 8d619e9f741a47f02a0ce7a6f1a00cb8d229848e Mon Sep 17 00:00:00 2001 From: Arcadio Quintero Date: Tue, 30 Jun 2026 16:54:51 -0400 Subject: [PATCH 17/18] test(edit-content): fix month-boundary day math in calendar TIME 'now' test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The calendar-field 'now' TIME default test compared getDate() day-of-month values directly (Math.abs(actualDay - expectedDay) <= 2). Across a month boundary the timezone-shifted UTC formValue rolls to the next month, so e.g. Jun 30 vs Jul 1 reads as |30 - 1| = 29 and fails — deterministically on month-end days (today is Jun 30). Compare the whole-day distance between the two dates instead, which is correct across month boundaries. Unrelated to the image-editor work; surfaced on this PR's CI run. --- .../calendar-field/calendar-field.util.spec.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-calendar-field/components/calendar-field/calendar-field.util.spec.ts b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-calendar-field/components/calendar-field/calendar-field.util.spec.ts index ec8f3db83f5..ad87bf88506 100644 --- a/core-web/libs/edit-content/src/lib/fields/dot-edit-content-calendar-field/components/calendar-field/calendar-field.util.spec.ts +++ b/core-web/libs/edit-content/src/lib/fields/dot-edit-content-calendar-field/components/calendar-field/calendar-field.util.spec.ts @@ -570,11 +570,18 @@ describe('DotEditContentCalendarFieldUtil - TDD Approach', () => { const todayInServerTz = getCurrentServerTime(SERVER_TIMEZONE_MOCKS.GULF); expect(result?.displayValue.getDate()).toBe(todayInServerTz.getDate()); - // formValue (UTC) might be different day due to timezone conversion, allow ±2 days - const expectedFormDate = todayInServerTz.getDate(); - const actualFormDate = result?.formValue.getDate(); - expect(actualFormDate).toBeDefined(); - expect(Math.abs(actualFormDate - expectedFormDate)).toBeLessThanOrEqual(2); + // formValue (UTC) might fall on a different calendar day due to timezone + // conversion; allow ±2 days. Compare the whole-day distance, not getDate() + // day-of-month subtraction — the latter wraps at month boundaries (e.g. + // Jun 30 vs Jul 1 would read as |30 - 1| = 29 instead of a 1-day difference). + const formValue = result?.formValue; + expect(formValue).toBeDefined(); + const dayMs = 24 * 60 * 60 * 1000; + const dayAtMidnight = (date: Date) => + new Date(date.getFullYear(), date.getMonth(), date.getDate()).getTime(); + const dayDistance = + Math.abs(dayAtMidnight(formValue!) - dayAtMidnight(todayInServerTz)) / dayMs; + expect(dayDistance).toBeLessThanOrEqual(2); }); it('should handle "now" defaultValue correctly for DATE fields', () => { From acce7c3a02a0b898b2a2771fc95ecf15122ff989 Mon Sep 17 00:00:00 2001 From: Adrian Molina Date: Wed, 1 Jul 2026 17:39:09 -0400 Subject: [PATCH 18/18] refactor(image-editor): enhance coalescing behavior for history entries Updated the coalesceHistory function to allow discrete actions (e.g., flips) to create individual history entries by passing a null coalesceKey. This change ensures that multiple flips of the same axis are recorded as separate undoable steps. Additionally, modified related tests and documentation to reflect the new behavior, ensuring legacy parity where a crop retains the resize state. Adjusted various tests to align with these changes, enhancing clarity and consistency in the image editor's functionality. --- .../store/features/with-crop.feature.spec.ts | 10 +++++--- .../lib/store/features/with-crop.feature.ts | 25 +++++++++---------- .../features/with-transform.feature.spec.ts | 15 +++++++++++ .../store/features/with-transform.feature.ts | 15 +++++------ .../src/lib/store/image-editor.store-utils.ts | 25 +++++++++++++------ .../src/lib/store/image-editor.store.spec.ts | 9 +++++-- .../src/lib/utils/dimensions.util.spec.ts | 8 +++--- .../src/lib/utils/dimensions.util.ts | 11 +++----- .../utils/image-filter-url.builder.spec.ts | 14 ++++++----- .../src/lib/utils/image-filter-url.builder.ts | 18 ++++++++----- 10 files changed, 92 insertions(+), 58 deletions(-) diff --git a/core-web/libs/image-editor/src/lib/store/features/with-crop.feature.spec.ts b/core-web/libs/image-editor/src/lib/store/features/with-crop.feature.spec.ts index fdc2954b791..5239bce9e15 100644 --- a/core-web/libs/image-editor/src/lib/store/features/with-crop.feature.spec.ts +++ b/core-web/libs/image-editor/src/lib/store/features/with-crop.feature.spec.ts @@ -10,7 +10,8 @@ import { CropState } from '../../models/image-editor.models'; import { imageEditorToolEvents } from '../image-editor.events'; import { initialImageEditorState } from '../image-editor.state'; -// Seed a pending resize so applying a crop can be shown to clear it. +// Seed a pending resize so applying a crop can be shown to PRESERVE it (legacy +// parity: a crop drawn on the scaled preview keeps the resize, chain Resize->Crop). const RESIZED = { ...initialImageEditorState, transform: { ...initialImageEditorState.transform, scale: 50, outputWidth: 500 } @@ -35,7 +36,7 @@ describe('withCrop', () => { runInInjectionContext(injector, () => (tool = injectDispatch(imageEditorToolEvents))); }); - it('applies a crop, clears resize, returns to move and adds a crop entry', () => { + it('applies a crop, preserves resize, returns to move and adds a crop entry', () => { tool.cropApplied({ x: 10, y: 10, w: 200, h: 150, active: false, aspect: null }); expect(store.crop()).toEqual({ @@ -46,8 +47,9 @@ describe('withCrop', () => { active: true, aspect: null }); - expect(store.transform().scale).toBe(100); - expect(store.transform().outputWidth).toBeNull(); + // Legacy parity: a crop keeps the resize (it was drawn on the scaled preview). + expect(store.transform().scale).toBe(50); + expect(store.transform().outputWidth).toBe(500); expect(store.activeTool()).toBe('move'); expect(store.history().at(-1)?.category).toBe('crop'); expect(store.previewStatus()).toBe('loading'); diff --git a/core-web/libs/image-editor/src/lib/store/features/with-crop.feature.ts b/core-web/libs/image-editor/src/lib/store/features/with-crop.feature.ts index c26055c1079..86f509f1d74 100644 --- a/core-web/libs/image-editor/src/lib/store/features/with-crop.feature.ts +++ b/core-web/libs/image-editor/src/lib/store/features/with-crop.feature.ts @@ -1,16 +1,23 @@ import { signalStoreFeature, type } from '@ngrx/signals'; import { on, withReducer } from '@ngrx/signals/events'; -import { CropState, ImageEditorState, TransformState } from '../../models/image-editor.models'; +import { CropState, ImageEditorState } from '../../models/image-editor.models'; import { imageEditorToolEvents } from '../image-editor.events'; import { initialCropState } from '../image-editor.state'; import { coalesceHistory, editableSlicesOf } from '../image-editor.store-utils'; /** - * Crop feature: applies or cancels a manual crop selection. Applying a crop is - * mutually exclusive with resize (it clears scale/output dims), returns to the - * move tool and records a coalesced history entry; cancelling clears the pending - * selection. + * Crop feature: applies or cancels a manual crop selection. Applying a crop + * returns to the move tool and records a coalesced history entry; cancelling + * clears the pending selection. + * + * Crop / resize interplay mirrors the legacy editor, which is intentionally + * ASYMMETRIC: applying a resize clears any crop (handled in `withTransform`), + * but applying a crop KEEPS the resize — the crop box is drawn on the already + * scaled preview, so its pixels are in the scaled image's space and the server + * runs `Resize` then `Crop` (see `buildFilterChain`). Clearing the scale here + * would drop the resize and re-apply the crop against the full-resolution + * original, cutting the wrong region. */ export function withCrop() { return signalStoreFeature( @@ -18,17 +25,9 @@ export function withCrop() { withReducer( on(imageEditorToolEvents.cropApplied, ({ payload }, state) => { const crop: CropState = { ...payload, active: true }; - // Crop and resize are mutually exclusive: applying a crop clears resize. - const transform: TransformState = { - ...state.transform, - scale: 100, - outputWidth: null, - outputHeight: null - }; const next: ImageEditorState = { ...state, crop, - transform, activeTool: 'move', previewStatus: 'loading', cacheBust: state.cacheBust + 1 diff --git a/core-web/libs/image-editor/src/lib/store/features/with-transform.feature.spec.ts b/core-web/libs/image-editor/src/lib/store/features/with-transform.feature.spec.ts index da0276cb831..612656f1c7e 100644 --- a/core-web/libs/image-editor/src/lib/store/features/with-transform.feature.spec.ts +++ b/core-web/libs/image-editor/src/lib/store/features/with-transform.feature.spec.ts @@ -83,6 +83,21 @@ describe('withTransform', () => { ]); }); + it('records each flip of the same axis as its own history step (discrete toggle)', () => { + const { store, transform } = setup(TransformStore); + transform.flipHToggled(); + transform.flipHToggled(); + // Two clicks net back to the un-flipped state... + expect(store.transform().flipH).toBe(false); + // ...but each click is a distinct undoable step — it must NOT coalesce into + // one silently-updated entry (the QA bug: applied twice, shown once). + expect(store.history().map((entry) => entry.label)).toEqual([ + 'Flip horizontal', + 'Flip horizontal' + ]); + expect(store.historyIndex()).toBe(1); + }); + it('clears an active crop when explicit output dimensions are set', () => { const { store, transform } = setup(TransformStoreCropped); transform.outputDimsChanged({ width: 500, height: null }); diff --git a/core-web/libs/image-editor/src/lib/store/features/with-transform.feature.ts b/core-web/libs/image-editor/src/lib/store/features/with-transform.feature.ts index c41d248643e..eda195d2a16 100644 --- a/core-web/libs/image-editor/src/lib/store/features/with-transform.feature.ts +++ b/core-web/libs/image-editor/src/lib/store/features/with-transform.feature.ts @@ -42,13 +42,16 @@ export function withTransform() { flipH: !state.transform.flipH }; + // A flip is a discrete toggle, not a continuous control: pass a null + // coalesceKey so each click is its own history step (two flips of the + // same axis are two entries, not one silently updated in place). return transformPatch( state, transform, state.crop, 'flip', 'Flip horizontal', - 'flipH' + null ); }), on(imageEditorTransformEvents.flipVToggled, (_event, state) => { @@ -57,14 +60,8 @@ export function withTransform() { flipV: !state.transform.flipV }; - return transformPatch( - state, - transform, - state.crop, - 'flip', - 'Flip vertical', - 'flipV' - ); + // Discrete toggle — see flipHToggled: null disables coalescing. + return transformPatch(state, transform, state.crop, 'flip', 'Flip vertical', null); }), on(imageEditorTransformEvents.outputDimsChanged, ({ payload }, state) => { const transform: TransformState = { diff --git a/core-web/libs/image-editor/src/lib/store/image-editor.store-utils.ts b/core-web/libs/image-editor/src/lib/store/image-editor.store-utils.ts index f14698e9898..f6208bef7ba 100644 --- a/core-web/libs/image-editor/src/lib/store/image-editor.store-utils.ts +++ b/core-web/libs/image-editor/src/lib/store/image-editor.store-utils.ts @@ -57,11 +57,17 @@ let historyEntrySeq = 0; * {@link FilterCategory} therefore starts a new entry rather than overwriting the * previous one. Either way a new value invalidates forward history built against * the old one. + * + * Passing a `null` coalesceKey marks a *discrete* action (e.g. a flip toggle, + * which is a button click, not a continuous drag): it never merges into the head, + * so every invocation is its own undoable step — two flips of the same axis are + * two history entries, not one silently updated in place. * @param state - The current editor state * @param category - The category the edit belongs to (used for grouping/labels) * @param label - The human-readable label for the entry * @param snapshot - The editable slices to capture - * @param coalesceKey - The control identity edits merge on; defaults to `category` + * @param coalesceKey - The control identity edits merge on; defaults to `category`. + * `null` disables coalescing (each call appends a distinct entry). * @returns The new `history` array and `historyIndex` */ export function coalesceHistory( @@ -69,11 +75,11 @@ export function coalesceHistory( category: FilterCategory, label: string, snapshot: EditableSlices, - coalesceKey: string = category + coalesceKey: string | null = category ): Pick { const head = state.history[state.historyIndex]; - if (head && head.coalesceKey === coalesceKey) { + if (coalesceKey !== null && head && head.coalesceKey === coalesceKey) { // Update the head in place AND drop any redo tail: a new value for the same // control invalidates forward history built against the old value (matching // the redo-tail truncation the new-entry branch below performs). @@ -85,12 +91,15 @@ export function coalesceHistory( return { history, historyIndex: state.historyIndex }; } + // A monotonic counter keeps ids collision-free even for two entries created + // in the same millisecond (Date.now() could collide, e.g. under fake timers). + const id = `${category}-${++historyEntrySeq}`; const entry: ImageEditorHistoryEntry = { - // A monotonic counter keeps ids collision-free even for two entries created - // in the same millisecond (Date.now() could collide, e.g. under fake timers). - id: `${category}-${++historyEntrySeq}`, + id, category, - coalesceKey, + // A discrete action (null key) falls back to its unique id so it never + // coalesces with a later action sharing the same category. + coalesceKey: coalesceKey ?? id, label, snapshot }; @@ -238,7 +247,7 @@ export function transformPatch( crop: CropState, category: FilterCategory, label: string, - coalesceKey: string = category + coalesceKey: string | null = category ): ImageEditorState { const next: ImageEditorState = { ...state, diff --git a/core-web/libs/image-editor/src/lib/store/image-editor.store.spec.ts b/core-web/libs/image-editor/src/lib/store/image-editor.store.spec.ts index 7008ce2ec72..c2a1b7cbe54 100644 --- a/core-web/libs/image-editor/src/lib/store/image-editor.store.spec.ts +++ b/core-web/libs/image-editor/src/lib/store/image-editor.store.spec.ts @@ -162,7 +162,7 @@ describe('ImageEditorStore', () => { expect(store.history()).toHaveLength(0); }); - it('should apply a crop, reset resize and add a crop entry', () => { + it('should apply a crop, preserve resize and add a crop entry', () => { transform.scaleChanged(50); tool.cropApplied({ x: 10, y: 10, w: 200, h: 150, active: false, aspect: null }); @@ -174,7 +174,12 @@ describe('ImageEditorStore', () => { active: true, aspect: null }); - expect(store.transform().scale).toBe(100); + // Legacy parity: a crop drawn on the scaled preview keeps the resize + // (the server runs Resize then Crop), so the scale must NOT reset. + // (The Resize->Crop chain ordering is covered in the builder spec, which + // has real natural dimensions; this store has none, so scale alone can't + // emit a Resize filter here.) + expect(store.transform().scale).toBe(50); expect(store.activeTool()).toBe('move'); expect(store.history().at(-1)?.category).toBe('crop'); }); diff --git a/core-web/libs/image-editor/src/lib/utils/dimensions.util.spec.ts b/core-web/libs/image-editor/src/lib/utils/dimensions.util.spec.ts index ea113dbeac9..edca0d70ed1 100644 --- a/core-web/libs/image-editor/src/lib/utils/dimensions.util.spec.ts +++ b/core-web/libs/image-editor/src/lib/utils/dimensions.util.spec.ts @@ -92,7 +92,7 @@ describe('dimensions.util', () => { }); }); - it('uses the crop size when an active crop is present and not resizing', () => { + it('uses the crop size when an active crop is present', () => { expect( computeOutputDimensions( ctx(1000, 800), @@ -102,14 +102,16 @@ describe('dimensions.util', () => { ).toEqual({ width: 200, height: 151 }); }); - it('ignores the crop when resizing (resize supersedes crop)', () => { + it('uses the crop size even when resizing (crop runs after resize)', () => { + // Legacy parity: the crop was drawn on the scaled preview, so its w/h are + // already in the resized space and dictate the final size. expect( computeOutputDimensions( ctx(1000, 800), transform({ scale: 50 }), crop({ active: true, w: 200, h: 150 }) ) - ).toEqual({ width: 500, height: 400 }); + ).toEqual({ width: 200, height: 150 }); }); it('uses explicit output width and height when both are set', () => { diff --git a/core-web/libs/image-editor/src/lib/utils/dimensions.util.ts b/core-web/libs/image-editor/src/lib/utils/dimensions.util.ts index 061960b4cdb..7dc6628eb2a 100644 --- a/core-web/libs/image-editor/src/lib/utils/dimensions.util.ts +++ b/core-web/libs/image-editor/src/lib/utils/dimensions.util.ts @@ -62,13 +62,10 @@ export function computeOutputDimensions( let width = ctx.naturalWidth; let height = ctx.naturalHeight; - const isResizing = - transform.outputWidth != null || transform.outputHeight != null || transform.scale !== 100; - - // Resizing supersedes crop, mirroring the filter-chain rule. - if (!isResizing && crop.active && crop.w > 0 && crop.h > 0) { - width = Math.round(crop.w); - height = Math.round(crop.h); + // A crop runs last in the chain (after any resize), so when one is active it + // dictates the final size — its w/h are already in the scaled image's space. + if (crop.active && crop.w > 0 && crop.h > 0) { + return { width: Math.round(crop.w), height: Math.round(crop.h) }; } if (transform.outputWidth != null || transform.outputHeight != null) { diff --git a/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.spec.ts b/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.spec.ts index 596404ce317..a86b2a83ae5 100644 --- a/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.spec.ts +++ b/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.spec.ts @@ -188,22 +188,24 @@ describe('image-filter-url.builder', () => { }); }); - describe('buildFilterChain - resize removes crop', () => { - it('drops the Crop filter when resizing', () => { + describe('buildFilterChain - resize and crop coexist (legacy parity)', () => { + it('keeps the Crop after the Resize when explicit output dimensions are set', () => { const result = chain({ transform: { outputWidth: 600 }, crop: { active: true, x: 0, y: 0, w: 100, h: 100 } }); - expect(result).toEqual([{ name: 'Resize', args: '/resize_w/600' }]); - expect(result.some((f) => f.name === 'Crop')).toBe(false); + // Crop is drawn on the scaled preview, so it runs after the resize. + expect(result.map((f) => f.name)).toEqual(['Resize', 'Crop']); }); - it('treats a non-100 scale as resizing and drops crop', () => { + it('keeps the Crop after a non-100 scale resize', () => { const result = chain({ transform: { scale: 50 }, crop: { active: true, x: 0, y: 0, w: 100, h: 100 } }); - expect(result.some((f) => f.name === 'Crop')).toBe(false); + expect(result.map((f) => f.name)).toEqual(['Resize', 'Crop']); + // The crop coords stay in the scaled space (they are emitted verbatim). + expect(result[1].args).toBe('/crop_w/100/crop_h/100/crop_x/0/crop_y/0'); }); it('builds a Resize filter from scale% × the natural size', () => { diff --git a/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.ts b/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.ts index f74c68018ed..20755c46015 100644 --- a/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.ts +++ b/core-web/libs/image-editor/src/lib/utils/image-filter-url.builder.ts @@ -44,10 +44,12 @@ function compressionFilter(mode: CompressionMode, quality: number): AppliedFilte /** * Builds the ordered list of server filters from the current edit state, - * mirroring the legacy ImageEditor rules: resizing removes crop, vertical flip - * is expressed as a 180deg rotation plus flip-token cancellation, and the - * compression filter is always applied last and exclusively. Crop is placed - * relative to the rotate/flip transforms by {@link FilterChainInput.cropBeforeTransforms}: + * mirroring the legacy ImageEditor rules: resize and crop coexist (the legacy + * rule is asymmetric — adding a resize clears any crop, but a crop drawn on the + * scaled preview keeps the resize and runs after it, so the crop pixels stay in + * the scaled image's space), vertical flip is expressed as a 180deg rotation plus + * flip-token cancellation, and the compression filter is always applied last and + * exclusively. Crop is placed relative to the rotate/flip transforms by {@link FilterChainInput.cropBeforeTransforms}: * a crop drawn before rotating must run first (on the un-rotated image it was drawn * against), while a crop drawn on an already-rotated preview runs after — so the * server applies it in the same coordinate space the user saw. @@ -74,9 +76,13 @@ export function buildFilterChain(input: FilterChainInput): AppliedFilter[] { filters.push({ name: 'Resize', args }); } - // Resize and crop stay mutually exclusive: resize wins. + // Resize and crop coexist: the crop box was drawn on the already-scaled + // preview, so its pixels are in the resized image's space. Emitting the Crop + // after the Resize (below) makes the server crop the scaled image — the region + // the user actually selected. (The reverse guard — a resize clearing an + // existing crop — lives in withTransform, mirroring the legacy editor.) const cropFilter = - !hasResize && crop.active && crop.w > 0 && crop.h > 0 + crop.active && crop.w > 0 && crop.h > 0 ? { name: 'Crop' as const, args: