Skip to content

Commit 937a563

Browse files
authored
fix: Handle no origins or invalid origin (#3025)
- Fixes edge case where profile without valid origin can be saved - Handles edge case of no saved sites, since backend does not currently prevent saving profiles without any origins
1 parent 1511f9b commit 937a563

File tree

5 files changed

+88
-25
lines changed

5 files changed

+88
-25
lines changed

frontend/src/features/browser-profiles/profile-browser-dialog.ts

Lines changed: 38 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import type { Dialog } from "@/components/ui/dialog";
1313
import {
1414
bgClass,
1515
type BrowserConnectionChange,
16+
type BrowserOriginsChange,
1617
type ProfileBrowser,
1718
} from "@/features/browser-profiles/profile-browser";
1819
import type {
@@ -24,6 +25,14 @@ import type { Profile } from "@/types/crawler";
2425
import { isApiError } from "@/utils/api";
2526
import { tw } from "@/utils/tailwind";
2627

28+
enum BrowserStatus {
29+
Initial,
30+
Pending,
31+
Ready,
32+
Complete,
33+
Error,
34+
}
35+
2736
/**
2837
* @fires btrix-updated
2938
*/
@@ -43,7 +52,7 @@ export class ProfileBrowserDialog extends BtrixElement {
4352
open = false;
4453

4554
@state()
46-
private isBrowserLoaded = false;
55+
private browserStatus = BrowserStatus.Initial;
4756

4857
@state()
4958
private showConfirmation = false;
@@ -77,7 +86,7 @@ export class ProfileBrowserDialog extends BtrixElement {
7786
if (changedProperties.has("open")) {
7887
if (!this.open) {
7988
this.showConfirmation = false;
80-
this.isBrowserLoaded = false;
89+
this.browserStatus = BrowserStatus.Initial;
8190
this.#savedBrowserId = undefined;
8291
this.browserIdTask.abort();
8392
}
@@ -142,6 +151,7 @@ export class ProfileBrowserDialog extends BtrixElement {
142151
render() {
143152
const isCrawler = this.appState.isCrawler;
144153
const creatingNew = this.duplicating || !this.profile;
154+
const incomplete = this.browserStatus !== BrowserStatus.Complete;
145155
const saving = this.saveProfileTask.status === TaskStatus.PENDING;
146156

147157
return html`<btrix-dialog
@@ -245,7 +255,9 @@ export class ProfileBrowserDialog extends BtrixElement {
245255
>
246256
<sl-button
247257
size="small"
248-
@click=${this.showConfirmation || !this.isBrowserLoaded
258+
@click=${this.showConfirmation ||
259+
this.browserStatus < BrowserStatus.Ready ||
260+
this.browserStatus === BrowserStatus.Error
249261
? () => void this.dialog?.hide()
250262
: () => (this.showConfirmation = true)}
251263
>
@@ -254,13 +266,13 @@ export class ProfileBrowserDialog extends BtrixElement {
254266
</btrix-popover>
255267
256268
<btrix-popover
257-
content=${msg("Save disabled during load.")}
258-
?disabled=${this.isBrowserLoaded}
269+
content=${msg("Disabled until page is finished loading")}
270+
?disabled=${!incomplete}
259271
>
260272
<sl-button
261273
size="small"
262274
variant="primary"
263-
?disabled=${!this.isBrowserLoaded || saving}
275+
?disabled=${incomplete || saving}
264276
?loading=${saving}
265277
@click=${() => void this.submit()}
266278
>
@@ -289,12 +301,16 @@ export class ProfileBrowserDialog extends BtrixElement {
289301
? html`<btrix-profile-browser
290302
browserId=${browserId}
291303
initialNavigateUrl=${ifDefined(this.config?.url)}
292-
.initialOrigins=${this.profile?.origins}
304+
.initialOrigins=${this.config?.profileId
305+
? this.profile?.origins
306+
: // Profile is being replaced if ID is not specified
307+
undefined}
293308
@btrix-browser-load=${this.onBrowserLoad}
294309
@btrix-browser-reload=${this.onBrowserReload}
295310
@btrix-browser-error=${this.onBrowserError}
296311
@btrix-browser-connection-change=${this
297312
.onBrowserConnectionChange}
313+
@btrix-browser-origins-change=${this.onBrowserOriginsChange}
298314
hideControls
299315
tabindex="0"
300316
.autofocus=${true}
@@ -306,25 +322,35 @@ export class ProfileBrowserDialog extends BtrixElement {
306322
}
307323

308324
private readonly closeBrowser = () => {
309-
this.isBrowserLoaded = false;
325+
this.browserStatus = BrowserStatus.Initial;
310326
};
311327

312328
private readonly onBrowserLoad = () => {
313-
this.isBrowserLoaded = true;
329+
this.browserStatus = BrowserStatus.Ready;
314330
};
315331

316332
private readonly onBrowserReload = () => {
317-
this.isBrowserLoaded = false;
333+
this.browserStatus = BrowserStatus.Pending;
318334
};
319335

320336
private readonly onBrowserError = () => {
321-
this.isBrowserLoaded = false;
337+
this.browserStatus = BrowserStatus.Error;
338+
};
339+
340+
private readonly onBrowserOriginsChange = (
341+
e: CustomEvent<BrowserOriginsChange>,
342+
) => {
343+
if (e.detail.origins.length) {
344+
this.browserStatus = BrowserStatus.Complete;
345+
}
322346
};
323347

324348
private readonly onBrowserConnectionChange = (
325349
e: CustomEvent<BrowserConnectionChange>,
326350
) => {
327-
this.isBrowserLoaded = e.detail.connected;
351+
this.browserStatus = e.detail.connected
352+
? BrowserStatus.Pending
353+
: BrowserStatus.Initial;
328354
};
329355

330356
private async submit() {

frontend/src/features/browser-profiles/profile-browser.ts

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { BtrixElement } from "@/classes/BtrixElement";
1111
import { emptyMessage } from "@/layouts/emptyMessage";
1212
import type { Profile } from "@/types/crawler";
1313
import { isApiError, type APIError } from "@/utils/api";
14+
import { isNotEqual } from "@/utils/is-not-equal";
1415
import { tw } from "@/utils/tailwind";
1516

1617
// Matches background of embedded browser
@@ -31,6 +32,9 @@ export type BrowserNotAvailableError = {
3132
export type BrowserConnectionChange = {
3233
connected: boolean;
3334
};
35+
export type BrowserOriginsChange = {
36+
origins: string[];
37+
};
3438

3539
const isPolling = (value: unknown): value is number => {
3640
return typeof value === "number";
@@ -52,6 +56,7 @@ const isPolling = (value: unknown): value is number => {
5256
* @fires btrix-browser-error
5357
* @fires btrix-browser-reload
5458
* @fires btrix-browser-connection-change
59+
* @fires btrix-browser-origins-change Origins list has changed
5560
* @cssPart base
5661
* @cssPart browser
5762
* @cssPart iframe
@@ -166,9 +171,24 @@ export class ProfileBrowser extends BtrixElement {
166171
if (!browser || isPolling(browser)) return;
167172

168173
try {
169-
const data = await this.pingBrowser(browser.id, signal);
174+
const { origins } = await this.pingBrowser(browser.id, signal);
175+
176+
if (
177+
this.originsTask.value &&
178+
origins &&
179+
isNotEqual(this.originsTask.value, origins)
180+
) {
181+
this.dispatchEvent(
182+
new CustomEvent<BrowserOriginsChange>(
183+
"btrix-browser-origins-change",
184+
{
185+
detail: { origins },
186+
},
187+
),
188+
);
189+
}
170190

171-
return data.origins;
191+
return origins || [];
172192
} catch (err) {
173193
if (isApiError(err) && err.details === "no_such_browser") {
174194
void this.onBrowserError();

frontend/src/features/browser-profiles/start-browser-dialog.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -300,9 +300,15 @@ export class StartBrowserDialog extends BtrixElement {
300300
}}
301301
>
302302
<sl-option value="">${msg("New Site")}</sl-option>
303-
<sl-divider></sl-divider>
304-
<sl-menu-label>${msg("Saved Sites")}</sl-menu-label>
305-
${profile.origins.map(option)}
303+
304+
${when(
305+
profile.origins.length,
306+
() => html`
307+
<sl-divider></sl-divider>
308+
<sl-menu-label>${msg("Saved Sites")}</sl-menu-label>
309+
${profile.origins.map(option)}
310+
`,
311+
)}
306312
${when(this.workflowOrigins.value, (seeds) =>
307313
seeds.length
308314
? html`

frontend/src/pages/org/browser-profiles-list.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,9 @@ export class BrowserProfilesList extends BtrixElement {
521521
(a, b) => (b && a && b > a ? b : a),
522522
data.created,
523523
) || data.created;
524+
const none = html`<sl-tooltip hoist content=${msg("None")}>
525+
<sl-icon name="slash" class="text-base text-neutral-400"></sl-icon>
526+
</sl-tooltip>`;
524527

525528
return html`
526529
<btrix-table-row
@@ -550,7 +553,7 @@ export class BrowserProfilesList extends BtrixElement {
550553
</btrix-tag-container>
551554
</btrix-table-cell>
552555
<btrix-table-cell>
553-
${originsWithRemainder(data.origins)}
556+
${data.origins.length ? originsWithRemainder(data.origins) : none}
554557
</btrix-table-cell>
555558
<btrix-table-cell>
556559
${this.localize.relativeDate(modifiedByAnyDate, { capitalize: true })}

frontend/src/pages/org/browser-profiles/profile.ts

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -394,13 +394,21 @@ export class BrowserProfilesProfilePage extends BtrixElement {
394394

395395
return when(
396396
this.profile,
397-
(profile) => html`
398-
<div class="relative">
399-
<ul class="divide-y rounded-lg border bg-white shadow-sm">
400-
${origins(profile)}
401-
</ul>
402-
</div>
403-
`,
397+
(profile) =>
398+
profile.origins.length
399+
? html`
400+
<div class="relative">
401+
<ul class="divide-y rounded-lg border bg-white shadow-sm">
402+
${origins(profile)}
403+
</ul>
404+
</div>
405+
`
406+
: panelBody({
407+
content: emptyMessage({
408+
message: msg("No saved sites yet"),
409+
detail: msg("Load a new URL to configure this profile."),
410+
}),
411+
}),
404412
originsSkeleton,
405413
);
406414
}

0 commit comments

Comments
 (0)