From bd34c1be449b30e56dcd37c71f557f239c6bb219 Mon Sep 17 00:00:00 2001 From: Marco <5818581+marcomura@users.noreply.github.com> Date: Wed, 12 Feb 2025 10:33:45 -0800 Subject: [PATCH] Better fix image rendering in Jira Cloud tickets' description and comments (#105) * better fix for images * fix typo --- src/ipc/prActions.ts | 4 - src/lib/ipc/fromUI/startWork.ts | 8 ++ .../startwork/startWorkWebviewController.ts | 45 ++++++++++ .../atlascode/startwork/StartWorkPage.tsx | 42 ++++++++- src/util/html.test.ts | 75 ---------------- src/util/html.ts | 15 ---- src/webviews/components/WebviewComponent.tsx | 18 ++++ .../issue/AbstractIssueEditorPage.tsx | 23 +---- .../components/issue/CommentComponent.tsx | 5 +- .../components/issue/CreateIssuePage.tsx | 4 +- .../components/issue/JiraIssuePage.tsx | 87 +++---------------- src/webviews/jiraIssueWebview.ts | 4 +- 12 files changed, 130 insertions(+), 200 deletions(-) delete mode 100644 src/util/html.test.ts delete mode 100644 src/util/html.ts diff --git a/src/ipc/prActions.ts b/src/ipc/prActions.ts index c8fd13c1..7b6bff3d 100644 --- a/src/ipc/prActions.ts +++ b/src/ipc/prActions.ts @@ -172,7 +172,3 @@ export interface GetImageAction extends Action { action: 'getImage'; url: string; } - -export function isGetImage(a: Action): a is GetImageAction { - return (a).action === 'getImage'; -} diff --git a/src/lib/ipc/fromUI/startWork.ts b/src/lib/ipc/fromUI/startWork.ts index cb0c76cd..6a2968bc 100644 --- a/src/lib/ipc/fromUI/startWork.ts +++ b/src/lib/ipc/fromUI/startWork.ts @@ -9,12 +9,14 @@ export enum StartWorkActionType { ClosePage = 'closePage', StartRequest = 'startRequest', OpenSettings = 'openSettings', + GetImage = 'getImage', } export type StartWorkAction = | ReducerAction | ReducerAction | ReducerAction + | ReducerAction | CommonAction; export interface StartRequestAction { @@ -31,3 +33,9 @@ export interface OpenSettingsAction { section?: ConfigSection; subsection?: ConfigSubSection; } + +export interface GetImageAction { + nonce: string; + url: string; + siteDetailsStringified: string; +} diff --git a/src/lib/webview/controller/startwork/startWorkWebviewController.ts b/src/lib/webview/controller/startwork/startWorkWebviewController.ts index d1543bb6..3bc8d239 100644 --- a/src/lib/webview/controller/startwork/startWorkWebviewController.ts +++ b/src/lib/webview/controller/startwork/startWorkWebviewController.ts @@ -6,6 +6,8 @@ import { CommonActionType } from '../../../ipc/fromUI/common'; import { StartWorkAction, StartWorkActionType } from '../../../ipc/fromUI/startWork'; import { WebViewID } from '../../../ipc/models/common'; import { CommonMessage, CommonMessageType } from '../../../ipc/toUI/common'; +// eslint-disable-next-line no-restricted-imports +import { Container } from '../../../../container'; import { BranchType, emptyStartWorkIssueMessage, @@ -158,6 +160,49 @@ export class StartWorkWebviewController implements WebviewController ({ title: { @@ -308,6 +313,33 @@ const StartWorkPage: React.FunctionComponent = () => { setTransition(inProgressTransitionGuess); }, [state.issue]); + const postMessageWithEventPromise = ( + send: StartWorkAction, + waitForEvent: string, + timeout: number, + nonce?: string, + ): Promise => { + controller.postMessage(send); + return OnMessageEventPromise(waitForEvent, timeout, nonce); + }; + + const fetchImage = async (url: string): Promise => { + const nonce = uuid.v4(); + return ( + await postMessageWithEventPromise( + { + type: StartWorkActionType.GetImage, + nonce: nonce, + url: url, + siteDetailsStringified: JSON.stringify(state.issue.siteDetails), + }, + 'getImageDone', + ConnectionTimeout, + nonce, + ) + ).imgData; + }; + return ( { - + {state.issue.siteDetails.baseApiUrl && ( + fetchImage(url)} + /> + )} diff --git a/src/util/html.test.ts b/src/util/html.test.ts deleted file mode 100644 index b0a62fdd..00000000 --- a/src/util/html.test.ts +++ /dev/null @@ -1,75 +0,0 @@ -import * as html from './html'; - -describe('html util', () => { - describe('fix relative URLs in html', () => { - it('replaceRelativeURLsWithAbsolute correctly replaces the relative URL', () => { - const baseUrl = 'https://www.domain.com/path1/path2/path3'; - const htmlText = '

'; - - const expectedHtml = - '

'; - - const fixedHtml = html.replaceRelativeURLsWithAbsolute(htmlText, baseUrl); - expect(fixedHtml).toBe(expectedHtml); - }); - - it('replaceRelativeURLsWithAbsolute correctly replaces all relative URLs', () => { - const baseUrl = 'https://www.domain.com/path1/path2/path3'; - const htmlText = - '

' + - '' + - '' + - "" + - "" + - '

'; - - const expectedHtml = - '

' + - '' + - '' + - "" + - "" + - '

'; - - const fixedHtml = html.replaceRelativeURLsWithAbsolute(htmlText, baseUrl); - expect(fixedHtml).toBe(expectedHtml); - }); - - it("replaceRelativeURLsWithAbsolute doesn't replaces absolute URLs", () => { - const baseUrl = 'https://www.domain.com/path1/path2/path3'; - const htmlText = '

'; - - const fixedHtml = html.replaceRelativeURLsWithAbsolute(htmlText, baseUrl); - expect(fixedHtml).toBe(htmlText); - }); - - it("if there aren't relative URLs, nothing changes", () => { - const baseUrl = 'https://www.domain.com/path1/path2/path3'; - const htmlText = '

'; - - const fixedHtml = html.replaceRelativeURLsWithAbsolute(htmlText, baseUrl); - expect(fixedHtml).toBe(htmlText); - }); - - it('last segment of the path is ignored unless it ends with a /', () => { - const htmlText = ''; - - expect(html.replaceRelativeURLsWithAbsolute(htmlText, 'https://www.domain.com/path1/path2')).toBe( - '', - ); - expect(html.replaceRelativeURLsWithAbsolute(htmlText, 'https://www.domain.com/path1/path2/')).toBe( - '', - ); - }); - - it('nullables are correctly handled', () => { - expect(html.replaceRelativeURLsWithAbsolute('', 'https://www.domain.com/')).toBe(''); - expect(html.replaceRelativeURLsWithAbsolute(null!, 'https://www.domain.com/')).toBe(null); - expect(html.replaceRelativeURLsWithAbsolute(undefined!, 'https://www.domain.com/')).toBe(undefined); - - expect(html.replaceRelativeURLsWithAbsolute('

', '')).toBe('

'); - expect(html.replaceRelativeURLsWithAbsolute('

', null!)).toBe('

'); - expect(html.replaceRelativeURLsWithAbsolute('

', undefined!)).toBe('

'); - }); - }); -}); diff --git a/src/util/html.ts b/src/util/html.ts deleted file mode 100644 index 0ac20adb..00000000 --- a/src/util/html.ts +++ /dev/null @@ -1,15 +0,0 @@ -const regex1 = / src="\/[^"]+/g; -const regex2 = / src='\/[^']+/g; - -export function replaceRelativeURLsWithAbsolute(renderedHtml: string, baseApiUrl: string): string | undefined { - if (!renderedHtml || !baseApiUrl) { - return renderedHtml; - } - - // The regex is searching for anything starting with ' src="/' which is 7 chars long, - // and we need to get the relative URL without including its first /, so anything after those 7 characters. - // Therefore, substring(7). - return renderedHtml - .replace(regex1, (x) => ` src=\"${new URL(x.substring(7), baseApiUrl).href}`) - .replace(regex2, (x) => ` src=\'${new URL(x.substring(7), baseApiUrl).href}`); -} diff --git a/src/webviews/components/WebviewComponent.tsx b/src/webviews/components/WebviewComponent.tsx index 8872c599..1597cba4 100644 --- a/src/webviews/components/WebviewComponent.tsx +++ b/src/webviews/components/WebviewComponent.tsx @@ -1,7 +1,9 @@ import * as React from 'react'; import { Action, LegacyPMFData } from '../../ipc/messaging'; import { OnMessageEventPromise } from '../../util/reactpromise'; +import { ConnectionTimeout } from '../../util/time'; import { darken, lighten, opacity } from './colors'; +import uuid from 'uuid'; interface VsCodeApi { postMessage(msg: {}): void; @@ -113,4 +115,20 @@ export abstract class WebviewComponent extends React. this._api.postMessage(send); return OnMessageEventPromise(waitForEvent, timeout, nonce); } + + protected async fetchImage(url: string): Promise { + const nonce = uuid.v4(); + return ( + await this.postMessageWithEventPromise( + { + action: 'getImage', + nonce: nonce, + url: url, + }, + 'getImageDone', + ConnectionTimeout, + nonce, + ) + ).imgData; + } } diff --git a/src/webviews/components/issue/AbstractIssueEditorPage.tsx b/src/webviews/components/issue/AbstractIssueEditorPage.tsx index 3f6f0387..41ec2c85 100644 --- a/src/webviews/components/issue/AbstractIssueEditorPage.tsx +++ b/src/webviews/components/issue/AbstractIssueEditorPage.tsx @@ -40,7 +40,6 @@ import { } from '../../../ipc/issueMessaging'; import { Action, HostErrorMessage, Message } from '../../../ipc/messaging'; import { ConnectionTimeout } from '../../../util/time'; -import { replaceRelativeURLsWithAbsolute } from '../../../util/html'; import { colorToLozengeAppearanceMap } from '../colors'; import * as FieldValidators from '../fieldValidators'; import * as SelectFieldHelper from '../selectFieldHelper'; @@ -372,7 +371,7 @@ export abstract class AbstractIssueEditorPage< } }, 100); - protected getInputMarkup(field: FieldUI, baseApiUrl: string, editmode: boolean = false): any { + protected getInputMarkup(field: FieldUI, editmode: boolean = false): any { switch (field.uiType) { case UIType.Input: { let validateFunc = this.getValidateFunction(field, editmode); @@ -402,12 +401,10 @@ export abstract class AbstractIssueEditorPage< let markup: React.ReactNode =

; if ((field as InputFieldUI).isMultiline) { - const html = this.state.fieldValues[`${field.key}.rendered`] || undefined; - const fixedHtml = replaceRelativeURLsWithAbsolute(html, baseApiUrl); markup = ( (await this.fetchUsers(input)).map((user) => ({ displayName: user.displayName, @@ -420,21 +417,7 @@ export abstract class AbstractIssueEditorPage< onSave={async (val: string) => { await this.handleInlineEdit(field, val); }} - fetchImage={async (url: string) => { - const nonce = uuid.v4(); - return ( - await this.postMessageWithEventPromise( - { - action: 'getImage', - nonce: nonce, - url: url, - }, - 'getImageDone', - ConnectionTimeout, - nonce, - ) - ).imgData; - }} + fetchImage={(img) => this.fetchImage(img)} /> ); } else { diff --git a/src/webviews/components/issue/CommentComponent.tsx b/src/webviews/components/issue/CommentComponent.tsx index dd5824fe..c19698a6 100644 --- a/src/webviews/components/issue/CommentComponent.tsx +++ b/src/webviews/components/issue/CommentComponent.tsx @@ -12,13 +12,11 @@ import React, { useState } from 'react'; import { DetailedSiteInfo } from '../../../atlclients/authInfo'; import { RenderedContent } from '../RenderedContent'; import { TextAreaEditor } from './TextAreaEditor'; -import { replaceRelativeURLsWithAbsolute } from '../../../util/html'; type Props = { siteDetails: DetailedSiteInfo; comment: JiraComment; isServiceDeskProject: boolean; - baseApiUrl: string; fetchUsers: (input: string) => Promise; onSave: (commentBody: string, commentId?: string, restriction?: CommentVisibility) => void; onDelete: (commentId: string) => void; @@ -29,7 +27,6 @@ export const CommentComponent: React.FC = ({ siteDetails, comment, isServiceDeskProject, - baseApiUrl, fetchUsers, onSave, onDelete, @@ -40,7 +37,7 @@ export const CommentComponent: React.FC = ({ const [isSaving, setIsSaving] = useState(false); const prettyCreated = `${formatDistanceToNow(parseISO(comment.created))} ago`; - const body = replaceRelativeURLsWithAbsolute(comment.renderedBody!, baseApiUrl) || comment.body; + const body = comment.renderedBody ? comment.renderedBody : comment.body; const type = isServiceDeskProject ? (comment.jsdPublic ? 'external' : 'internal') : undefined; if (editing && !isSaving) { diff --git a/src/webviews/components/issue/CreateIssuePage.tsx b/src/webviews/components/issue/CreateIssuePage.tsx index 671bb347..a78c0c2a 100644 --- a/src/webviews/components/issue/CreateIssuePage.tsx +++ b/src/webviews/components/issue/CreateIssuePage.tsx @@ -278,11 +278,11 @@ export default class CreateIssuePage extends AbstractIssueEditorPage this.getInputMarkup(field, this.state.siteDetails.baseApiUrl)); + return this.commonFields.map((field) => this.getInputMarkup(field)); } getAdvancedFieldMarkup(): any { - return this.advancedFields.map((field) => this.getInputMarkup(field, this.state.siteDetails.baseApiUrl)); + return this.advancedFields.map((field) => this.getInputMarkup(field)); } public render() { diff --git a/src/webviews/components/issue/JiraIssuePage.tsx b/src/webviews/components/issue/JiraIssuePage.tsx index db1b7173..cbfa6261 100644 --- a/src/webviews/components/issue/JiraIssuePage.tsx +++ b/src/webviews/components/issue/JiraIssuePage.tsx @@ -79,12 +79,12 @@ export default class JiraIssuePage extends AbstractIssueEditorPage { @@ -501,14 +501,7 @@ export default class JiraIssuePage extends AbstractIssueEditorPage -

- {this.getInputMarkup( - this.state.fields['summary'], - this.state.siteDetails.baseApiUrl, - true, - 'summary', - )} -

+

{this.getInputMarkup(this.state.fields['summary'], true, 'summary')}

{this.state.isErrorBannerOpen && ( @@ -523,12 +516,7 @@ export default class JiraIssuePage extends AbstractIssueEditorPage - {this.getInputMarkup( - this.state.fields['description'], - this.state.siteDetails.baseApiUrl, - true, - 'description', - )} + {this.getInputMarkup(this.state.fields['description'], true, 'description')} )} {this.state.fields['attachment'] && @@ -540,21 +528,7 @@ export default class JiraIssuePage extends AbstractIssueEditorPage { - const nonce = uuid.v4(); - return ( - await this.postMessageWithEventPromise( - { - action: 'getImage', - nonce: nonce, - url: url, - }, - 'getImageDone', - ConnectionTimeout, - nonce, - ) - ).imgData; - }} + fetchImage={(img) => this.fetchImage(img)} /> )} @@ -564,12 +538,7 @@ export default class JiraIssuePage extends AbstractIssueEditorPage - {this.getInputMarkup( - this.state.fields['environment'], - this.state.siteDetails.baseApiUrl, - true, - 'environment', - )} + {this.getInputMarkup(this.state.fields['environment'], true, 'environment')} )} @@ -585,12 +554,7 @@ export default class JiraIssuePage extends AbstractIssueEditorPage - {this.getInputMarkup( - this.state.fields['subtasks'], - this.state.siteDetails.baseApiUrl, - true, - 'subtasks', - )} + {this.getInputMarkup(this.state.fields['subtasks'], true, 'subtasks')} - {this.getInputMarkup( - this.state.fields['issuelinks'], - this.state.siteDetails.baseApiUrl, - true, - 'issuelinks', - )} + {this.getInputMarkup(this.state.fields['issuelinks'], true, 'issuelinks')} { - const nonce = uuid.v4(); - return ( - await this.postMessageWithEventPromise( - { - action: 'getImage', - nonce: nonce, - url: url, - }, - 'getImageDone', - ConnectionTimeout, - nonce, - ) - ).imgData; - }} + fetchImage={(img) => this.fetchImage(img)} /> ))} - {this.getInputMarkup( - this.state.fields['comment'], - this.state.siteDetails.baseApiUrl, - true, - 'comment', - )} + {this.getInputMarkup(this.state.fields['comment'], true, 'comment')} )} @@ -847,7 +786,7 @@ export default class JiraIssuePage extends AbstractIssueEditorPage - {this.getInputMarkup(field, this.state.siteDetails.baseApiUrl, true, key)} + {this.getInputMarkup(field, true, key)} ) ); @@ -864,7 +803,7 @@ export default class JiraIssuePage extends AbstractIssueEditorPage - {this.getInputMarkup(field, this.state.siteDetails.baseApiUrl, true, `Advanced sidebar`)} + {this.getInputMarkup(field, true, `Advanced sidebar`)} , ); } @@ -906,7 +845,7 @@ export default class JiraIssuePage extends AbstractIssueEditorPage - {this.getInputMarkup(field, this.state.siteDetails.baseApiUrl, true, `Advanced main`)} + {this.getInputMarkup(field, true, `Advanced main`)} , ); } diff --git a/src/webviews/jiraIssueWebview.ts b/src/webviews/jiraIssueWebview.ts index 1450ef4a..7f86232a 100644 --- a/src/webviews/jiraIssueWebview.ts +++ b/src/webviews/jiraIssueWebview.ts @@ -918,8 +918,8 @@ export class JiraIssueWebview ), ); // Prefix base URL for a relative URL - const href = msg.url.startsWith('/secure/attachment') - ? new URL(baseApiUrl + msg.url) + const href = msg.url.startsWith('/') + ? new URL(baseApiUrl.href + msg.url) : new URL(msg.url); // Skip fetching external images (that do not belong to the site) if (href.hostname !== baseApiUrl.hostname) {