Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
"buffer": "^6.0.3",
"bulma": "^0.9.3",
"client-zip": "^2.3.0",
"flexsearch": "0.7.31",
"hash-wasm": "^4.9.0",
"http-status-codes": "^2.1.4",
"idb": "^7.1.1",
Expand Down
126 changes: 117 additions & 9 deletions src/argo-archive-list.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { LitElement, html, css, CSSResultGroup } from "lit";
import { customElement, state } from "lit/decorators.js";
import { LitElement, html, css, CSSResultGroup, PropertyValues } from "lit";
import { customElement, state, property } from "lit/decorators.js";
import { styles as typescaleStyles } from "@material/web/typography/md-typescale-styles.js";

import "@material/web/list/list.js";
Expand All @@ -9,6 +9,7 @@ import "@material/web/icon/icon.js";
import "@material/web/labs/card/elevated-card.js";

import { getLocalOption } from "./localstorage";
import { Index as FlexIndex } from "flexsearch";

@customElement("argo-archive-list")
export class ArgoArchiveList extends LitElement {
Expand Down Expand Up @@ -109,6 +110,22 @@ export class ArgoArchiveList extends LitElement {
flex-shrink: 0;
text-decoration: none;
}
.search-result-text {
width: 100%;
padding-left: 14px;
padding-right: 12px;
padding-top: 4px;
padding-bottom: 12px;
box-sizing: border-box;
}

.search-result-text b {
background-color: #cf7df1;
color: black;
font-weight: bold;
padding: 0 2px;
border-radius: 2px;
}
`,
];

Expand All @@ -118,9 +135,54 @@ export class ArgoArchiveList extends LitElement {
url: string;
title?: string;
favIconUrl?: string;
text?: string;
}> = [];
@state() private collId = "";
@state() private selectedPages = new Set<string>();
@state() private filteredPages = [] as typeof this.pages;

@property({ type: String }) filterQuery = "";
private flex: FlexIndex<string> = new FlexIndex<string>({
tokenize: "forward",
resolution: 3,
});

protected updated(changed: PropertyValues) {
super.updated(changed);

// 2) Rebuild the index when the raw pages change:
if (changed.has("pages")) {
this.flex = new FlexIndex<string>({
tokenize: "forward",
resolution: 3,
});
this.pages.forEach((p) => {
// include title + text (and URL if you like)

const toIndex = [p.title ?? "", p.text ?? ""].join(" ");
this.flex.add(p.ts, toIndex);
});
}

// 3) Whenever pages or the query change, recompute filteredPages:
if (changed.has("pages") || changed.has("filterQuery")) {
if (!this.filterQuery.trim()) {
this.filteredPages = [...this.pages];
} else {
// partial matches on title/text via the “match” preset
const matches = this.flex.search(this.filterQuery) as string[];
this.filteredPages = this.pages.filter((p) => matches.includes(p.ts));
}
}
}

private buildIndex() {
this.flex = new FlexIndex();
this.pages.forEach((p) => {
const text = p.url + (p.title ? ` ${p.title}` : "");
this.flex.add(p.ts, text); // use ts (timestamp) as a unique id
});
}
Comment on lines +179 to 185

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The buildIndex() method is defined but never called in the code. Additionally, it creates a new index without the same configuration options used in the updated() method, and it doesn't include the page text in the indexed content, unlike the implementation in updated(). [possible issue, importance: 7]

Suggested change
private buildIndex() {
this.flex = new FlexIndex();
this.pages.forEach((p) => {
const text = p.url + (p.title ? ` ${p.title}` : "");
this.flex.add(p.ts, text); // use ts (timestamp) as a unique id
});
}
private buildIndex() {
this.flex = new FlexIndex<string>({
tokenize: "forward",
resolution: 3,
});
this.pages.forEach((p) => {
const toIndex = [p.title ?? "", p.text ?? "", p.url].join(" ");
this.flex.add(p.ts, toIndex);
});
}


private togglePageSelection(ts: string) {
const next = new Set(this.selectedPages);
Expand Down Expand Up @@ -155,18 +217,37 @@ export class ArgoArchiveList extends LitElement {
});
}

private _highlightMatch(
text?: string,
query: string = "",
maxLen = 180,
): string {
if (!text) return "";

const safeQuery = query.trim().replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
const regex = new RegExp(safeQuery, "ig");

const matchIndex = text.search(regex);
if (matchIndex === -1) return text.slice(0, maxLen) + "...";

const previewStart = Math.max(0, matchIndex - 30);
const preview = text.slice(previewStart, previewStart + maxLen);

return preview.replace(regex, (m) => `<b>${m}</b>`) + "...";
}
Comment on lines +220 to +237

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The current implementation is vulnerable to XSS attacks because it directly sets innerHTML with user-controlled content. Even though you're escaping regex special characters, HTML special characters aren't being escaped, which could allow injection of malicious HTML. [security, importance: 9]

Suggested change
private _highlightMatch(
text?: string,
query: string = "",
maxLen = 180,
): string {
if (!text) return "";
const safeQuery = query.trim().replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
const regex = new RegExp(safeQuery, "ig");
const matchIndex = text.search(regex);
if (matchIndex === -1) return text.slice(0, maxLen) + "...";
const previewStart = Math.max(0, matchIndex - 30);
const preview = text.slice(previewStart, previewStart + maxLen);
return preview.replace(regex, (m) => `<b>${m}</b>`) + "...";
}
private _highlightMatch(
text?: string,
query: string = "",
maxLen = 180,
): string {
if (!text) return "";
const safeQuery = query.trim().replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
const regex = new RegExp(safeQuery, "ig");
const matchIndex = text.search(regex);
if (matchIndex === -1) return text.slice(0, maxLen) + "...";
const previewStart = Math.max(0, matchIndex - 30);
const preview = text.slice(previewStart, previewStart + maxLen);
// Escape HTML special characters first
const escapedPreview = preview.replace(/[&<>"']/g, (m) => {
return {'&': '&amp;', '<': '&lt;', '>': '&gt;', '"': '&quot;', "'": '&#39;'}[m] || m;
});
return escapedPreview.replace(regex, (m) => `<b>${m}</b>`) + "...";
}


render() {
if (!this.pages.length) {
return html`<p class="md-typescale-body-medium">No archives yet.</p>`;
}

const groups = this.pages.reduce(
const groups = this.filteredPages.reduce(
(acc, page) => {
const key = this._formatDate(new Date(Number(page.ts)));
(acc[key] ||= []).push(page);
return acc;
},
{} as Record<string, typeof this.pages>,
{} as Record<string, typeof this.filteredPages>,
);

return html`
Expand Down Expand Up @@ -226,6 +307,20 @@ export class ArgoArchiveList extends LitElement {
>
</div>
</md-list-item>
${this.filterQuery && page.text
? html`
<div
class="search-result-text md-typescale-body-small"
>
<span
.innerHTML=${this._highlightMatch(
page.text,
this.filterQuery,
)}
></span>
</div>
`
: ""}
`;
})}
</md-list>
Expand Down Expand Up @@ -254,15 +349,28 @@ export class ArgoArchiveList extends LitElement {
return label;
}

private _openPage(page: { ts: string; url: string }) {
private async _openPage(page: { ts: string; url: string }) {
const tsParam = new Date(Number(page.ts))
.toISOString()
.replace(/[-:TZ.]/g, "");
const urlEnc = encodeURIComponent(page.url);
const fullUrl =
`${chrome.runtime.getURL("index.html")}?source=local://${
this.collId
}&url=${urlEnc}` + `#view=pages&url=${urlEnc}&ts=${tsParam}`;
chrome.tabs.create({ url: fullUrl });
`${chrome.runtime.getURL("index.html")}?source=local://${this.collId}` +
`&url=${urlEnc}#view=pages&url=${urlEnc}&ts=${tsParam}`;

const extensionUrlPrefix = chrome.runtime.getURL("index.html");

// Check if any existing tab already displays the archive viewer
const tabs = await chrome.tabs.query({});
// @ts-expect-error - t implicitly has an 'any' type
const viewerTab = tabs.find((t) => t.url?.startsWith(extensionUrlPrefix));

if (viewerTab && viewerTab.id) {
// Reuse the existing tab
chrome.tabs.update(viewerTab.id, { url: fullUrl, active: true });
} else {
// Fallback: open a new tab
chrome.tabs.create({ url: fullUrl });
}
}
}
21 changes: 16 additions & 5 deletions src/ext/bg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,17 +115,22 @@ function sidepanelHandler(port) {
defaultCollId = message.collId;
autorun = message.autorun;

// @ts-expect-error - tabs doesn't have type definitions
chrome.tabs.query(
{ active: true, currentWindow: true },
//@ts-expect-error tabs has any type
async (tabs) => {
for (const tab of tabs) {
if (!isValidUrl(tab.url)) continue;

// @ts-expect-error - TS2554 - Expected 2 arguments, but got 3.
await startRecorder(
tab.id,
{ collId: defaultCollId, port: null, autorun },
{
// @ts-expect-error - collId implicitly has an 'any' type.
collId: defaultCollId,
port: null,
autorun,
},
//@ts-expect-error - 2 parameters but 3
tab.url,
);
}
Expand Down Expand Up @@ -232,10 +237,16 @@ chrome.tabs.onActivated.addListener(async ({ tabId }) => {

if (!isValidUrl(tab.url)) return;
if (!self.recorders[tabId]) {
// @ts-expect-error - TS2554 - Expected 2 arguments, but got 3.
await startRecorder(
tabId,
{ collId: defaultCollId, port: null, autorun },
{
// @ts-expect-error - collId implicitly has an 'any' type.
collId: defaultCollId,
port: null,
autorun,
},

// @ts-expect-error - 2 parameters but 3
tab.url,
);
Comment on lines 240 to 251

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The code is passing three parameters to startRecorder() but adding a TypeScript error comment indicating it expects only two. This suggests a mismatch between the function signature and its usage. Either the function should be updated to accept three parameters, or the third parameter should be removed from all calls. [possible issue, importance: 8]

Suggested change
await startRecorder(
tabId,
{ collId: defaultCollId, port: null, autorun },
{
// @ts-expect-error - collId implicitly has an 'any' type.
collId: defaultCollId,
port: null,
autorun,
},
// @ts-expect-error - 2 parameters but 3
tab.url,
);
await startRecorder(
tabId,
{
collId: defaultCollId,
port: null,
autorun,
url: tab.url, // Include URL in the options object instead of as a separate parameter
}
);

}
Expand Down
24 changes: 21 additions & 3 deletions src/sidepanel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import "@material/web/button/outlined-button.js";
import "@material/web/divider/divider.js";
import { mapIntegerToRange, truncateString } from "./utils";
import { CollectionLoader } from "@webrecorder/wabac/swlib";
import WebTorrent from "webtorrent";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this not needed? @nikitalokhmachev-ai

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yamijuan No since we have a global import.


document.adoptedStyleSheets.push(typescaleStyles.styleSheet!);

Expand Down Expand Up @@ -129,7 +128,8 @@ class ArgoViewer extends LitElement {
private archiveList!: ArgoArchiveList;
constructor() {
super();

// @ts-expect-error - TS2339 - Property 'searchQuery' does not exist on type 'ArgoViewer'.
this.searchQuery = "";
// @ts-expect-error - TS2339 - Property 'collections' does not exist on type 'ArgoViewer'.
this.collections = [];
// @ts-expect-error - TS2339 - Property 'collTitle' does not exist on type 'ArgoViewer'.
Expand Down Expand Up @@ -183,6 +183,7 @@ class ArgoViewer extends LitElement {

static get properties() {
return {
searchQuery: { type: String },
collections: { type: Array },
collId: { type: String },
collTitle: { type: String },
Expand Down Expand Up @@ -721,6 +722,12 @@ class ArgoViewer extends LitElement {
<p>${this.notRecordingMessage}</p>`;
}

private onSearchInput(e: InputEvent) {
const input = e.currentTarget as HTMLInputElement;
// @ts-expect-error - TS2339 - Property 'searchQuery' does not exist on type 'ArgoViewer'.
this.searchQuery = input.value;
}

renderSearch() {
return html`
<div class="search-container">
Expand All @@ -729,6 +736,11 @@ class ArgoViewer extends LitElement {
placeholder="Search archived pages"
aria-label="Search archived pages"
class="search-field"
@input=${this.onSearchInput}
.value=${
// @ts-expect-error - TS2339 - Property 'searchQuery' does not exist on type 'ArgoViewer'.
this.searchQuery
}
>
<md-icon slot="leading-icon">search</md-icon>
</md-filled-text-field>
Expand All @@ -752,7 +764,13 @@ class ArgoViewer extends LitElement {
style="flex: 1; overflow-y: auto; position: relative; flex-grow: 1;"
>
<div id="my-archives" class="tab-panel" active>
<argo-archive-list id="archive-list"></argo-archive-list>
<argo-archive-list
id="archive-list"
.filterQuery=${
//@ts-expect-error - TS2339 - Property 'searchQuery' does not exist on type 'ArgoViewer'.
this.searchQuery
}
></argo-archive-list>
</div>
Comment on lines 764 to 774

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Instead of using TypeScript error suppressions for the missing searchQuery property, properly define it in the class properties. This will make the code more maintainable and type-safe. [general, importance: 3]

New proposed code:
 <div
   class="tab-panels"
   style="flex: 1; overflow-y: auto; position: relative; flex-grow: 1;"
 >
   <div id="my-archives" class="tab-panel" active>
     <argo-archive-list
       id="archive-list"
-      .filterQuery=${
-        //@ts-expect-error - TS2339 - Property 'searchQuery' does not exist on type 'ArgoViewer'.
-        this.searchQuery
-      }
+      .filterQuery=${this.searchQuery}
     ></argo-archive-list>
   </div>

<div id="shared-archives" class="tab-panel">
<!-- future “shared” list… -->
Expand Down
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4841,6 +4841,11 @@ flatted@^3.2.9:
resolved "https://registry.yarnpkg.com/flatted/-/flatted-3.3.1.tgz#21db470729a6734d4997002f439cb308987f567a"
integrity sha512-X8cqMLLie7KsNUDSdzeN8FYK9rEt4Dt67OsG/DNGnYTSDBG4uFAJFBnUeiV+zCVAvwFy56IjM9sH51jVaEhNxw==

flexsearch@0.7.31:
version "0.7.31"
resolved "https://registry.yarnpkg.com/flexsearch/-/flexsearch-0.7.31.tgz#065d4110b95083110b9b6c762a71a77cc52e4702"
integrity sha512-XGozTsMPYkm+6b5QL3Z9wQcJjNYxp0CYn3U1gO7dwD6PAqU1SVWZxI9CCg3z+ml3YfqdPnrBehaBrnH2AGKbNA==

flexsearch@^0.7.31:
version "0.7.43"
resolved "https://registry.yarnpkg.com/flexsearch/-/flexsearch-0.7.43.tgz#34f89b36278a466ce379c5bf6fb341965ed3f16c"
Expand Down
Loading