Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to Manifest V3 #131

Closed
steph-nb opened this issue Jan 10, 2023 · 9 comments · Fixed by #136
Closed

Switch to Manifest V3 #131

steph-nb opened this issue Jan 10, 2023 · 9 comments · Fixed by #136
Assignees

Comments

@steph-nb
Copy link

What about switching to manifest V3?
See:
https://developer.chrome.com/docs/extensions/mv3/intro/

As of January 17, 2022 the Chrome Web Store has stopped accepting new Manifest V2 extensions. We strongly recommend migrating your extensions to Manifest V3 as soon as possible; this will become mandatory after Manifest V2 is phased out in 2024.

@ikreymer
Copy link
Member

Yes, this is planned, eventually. I've done some tests and mostly it works, since already using a service worker.
There are some questions that remain regarding the debugger being attached (https://bugs.chromium.org/p/chromium/issues/detail?id=1139096&q=chrome.debugger%20manifest%20v3&can=2) and the CSP policy (which is currently bypassed for replay)

ikreymer added a commit that referenced this issue Jan 19, 2023
rename chrome.browserAction -> chrome.action
include ext/bg.js in sw/main.js
(addresses #131)
ikreymer added a commit that referenced this issue Jan 19, 2023
rename chrome.browserAction -> chrome.action
include ext/bg.js in sw/main.js
(addresses #131)
ikreymer added a commit that referenced this issue Feb 4, 2023
rename chrome.browserAction -> chrome.action
include ext/bg.js in sw/main.js (addresses #131)

move replay root to ./ from ./replay/
dynamically import sw.js from bg.js, use bg.js as service-worker entrypoint
ipfs: add Origin header overrides via declarative-net-request rules, also add as permission
via brave-ipfs.json rules

bump version to 0.10.0
@tsemachh
Copy link

tsemachh commented Aug 31, 2023

Hi @ikreymer
We try now to push our extension to the chrome store and it does require Manifest 3 only (New extension)
Want to share few obstacles we are facing regarding usage of eval in the code:

  1. One of the libraries used by WebRecorder is protobufjs there's issue opened there I also asked it to be promoted:
    https://github.com/protobufjs/protobuf.js/issues/1754 - It's being handled there , there's a PR it's not merged yet

  2. In the original recorder there's a code with eval which loads the pdf.js library:
    src/extractPDF.js
    const res = await fetch(new URL("pdf/pdf.min.js", baseUrl).href);\n eval(await res.text())
    From some reason(lazy load) it loads it using eval , it can be changed to be loaded using import.
    (Even though it's not really a security issue since it loads only local file)

Will update if we face more issue as I said we are in the process now

@tsemachh
Copy link

Note the protobuf is consumed from IPFS Utils which uses @ipld/unixfs and there's also an issue for it:
ipld/js-unixfs#31
Seems like everyone are trying to get rid of it , but it ain't going no where
Will update if I see progress

@ikreymer
Copy link
Member

Hi @ikreymer We try now to push our extension to the chrome store and it does require Manifest 3 only (New extension) Want to share few obstacles we are facing regarding usage of eval in the code:

To clarify, are these errors you're encountering in the extension, or comments from the chrome web store submission process?

The MV3 branch does build and run, though haven't tested it as extensively yet, and only in dev builds.

Can you put the fork that you're using up on GitHub somewhere, so we can be more clear what you're referring to?

@tsemachh
Copy link

tsemachh commented Sep 13, 2023 via email

@tsemachh
Copy link

We faced issues with upgrading to Yarn 3.* ,we got only in the production build next error:

typeError: Super constructor null of Fe is not a constructor
    at new Fe (index.js:6:5394)
    at e.serialize (index.js:6:6077)
    at q_.createWARCInfo (downloader.js:604:41)
    at async q_.generateWARC (CustomDownloader.js:76:15)
    at async SE (downloader.js:44:18)
    at async Object.pull (index.js:1:1384)

I digged it and it seems like error in the yarn3 with the webpack(Tried also upgrading webpack didn't help)
I did see similar issue at https://stackoverflow.com/questions/43176006/typeerror-class-extends-value-undefined-is-not-a-function-or-null
The problem I face is that:
warcio/src/lib/warcserializer.ts
BaseSerializerBuffer is null in the webpack build

// ===========================================================================
/* Base class for custom buffering while serializing */
export abstract class BaseSerializerBuffer {
  abstract write(chunk: Uint8Array): void;
  abstract readAll() : AsyncIterable<Uint8Array>;
}

// ===========================================================================
export class SerializerInMemBuffer extends BaseSerializerBuffer
{
  buffers: Array<Uint8Array> = [];

  write(chunk: Uint8Array): void {
    this.buffers.push(chunk);
  }

  async* readAll(): AsyncIterable<Uint8Array> {
    for (const buff of this.buffers) {
      yield buff;
    }
  }
}

For now I will try to go back to yarn 2 and see if it helps , will update

@AntiMoron

This comment was marked as spam.

@Shrinks99 Shrinks99 changed the title Switch to chrome Manifest V3? Switch to Manifest V3 May 29, 2024
@ikreymer ikreymer self-assigned this May 29, 2024
@Shrinks99 Shrinks99 moved this from Triage to Ready in Webrecorder Projects May 29, 2024
@ikreymer
Copy link
Member

ikreymer commented Jun 1, 2024

This is generally working, except for the protobuf issue ipld/js-unixfs#31. As a temporary solution, so we can migrate, may need to drop IPFS serialization on first MV3 release.

Edit: Hm, this may actually be working with latest @ipld/unixfs

@tsemachh
Copy link

tsemachh commented Jun 1, 2024

We finally got it approved but with the changes I mentioned before aka next patches:
https://github.com/shefing/web_recorder_ext_public/tree/main/patches

Not sure if approval for existing extension will be easier than new extensions.

I will check with Riki who manages the project if she encountered other issues.
(Unless she gave birth , it should be any day now)

ikreymer added a commit that referenced this issue Jun 4, 2024
- the extension is now served from chrome-extension://<id>/index.html instead of chrome-extension://<id>/replay/index.html as the service worker registration is no longer needed.
- Added check to uninstall old service worker from ./replay/sw.js as new service worker is served from ./bg.js
- sw registration removed (unless in embed mode), now registered automatically
- CSP disabling allows for PDF text extraction + ruffle to work as before, and still needed for replay
- remove brave IPFS support, regular IPFS support via auto-js-ipfs remains, including custom serialization via ipld/car
- dependencies: update to latest awp-sw
- embed fixes: fix window size, remove dist files from git, use http-server to serve embed page.

Fixes #131
@github-project-automation github-project-automation bot moved this from Ready to Done! in Webrecorder Projects Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done!
Development

Successfully merging a pull request may close this issue.

4 participants