Skip to content

Commit

Permalink
Fix Lite to work on FireFox (#9528)
Browse files Browse the repository at this point in the history
* Fix CrossOriginWorker to work in FireFox

* add changeset

* Add FireFox to the Lite E2E test

* Use request.arrayBuffer instead .body on FireFox

* add changeset

* Delete the user-agent header in PyodideHttpTransport

* Ignore the kitchen_sink E2E test for FireFox

* Fix

* Fix test files using file uploader so they work on FireFox, ref: https://stackoverflow.com/a/78701710

* Fix js/spa/test/outbreak_forecast.spec.ts

* Fix outbreak_forecast.spec.ts

* [wip] Comment out plotly part

* add changeset

* Skip Plotly tests on FireFox temporarily

* Revert "Fix"

This reverts commit 98e2495.

* Fix

* add changeset

---------

Co-authored-by: gradio-pr-bot <[email protected]>
Co-authored-by: Abubakar Abid <[email protected]>
  • Loading branch information
3 people authored Oct 10, 2024
1 parent 67e4044 commit 9004b11
Show file tree
Hide file tree
Showing 9 changed files with 85 additions and 26 deletions.
8 changes: 8 additions & 0 deletions .changeset/plenty-lizards-cheat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"@gradio/lite": patch
"@gradio/wasm": patch
"@self/tootils": patch
"gradio": patch
---

fix:Fix Lite to work on FireFox
13 changes: 12 additions & 1 deletion .config/playwright.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,18 @@ const lite = defineConfig(base, {
],
workers: 1,
retries: 3,
timeout: 60000
timeout: 60000,
projects: [
{
name: "chromium",
use: { ...devices["Desktop Chrome"] }
},
{
name: "firefox",
use: { ...devices["Desktop Firefox"] },
testIgnore: "**/kitchen_sink.*" // This test requires the camera permission but it's not supported on FireFox: https://github.com/microsoft/playwright/issues/11714
}
]
});

export default !!process.env.GRADIO_E2E_TEST_LITE ? lite : normal;
6 changes: 6 additions & 0 deletions gradio/processing_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,13 @@ async def handle_async_request(
) -> httpx.Response:
url = str(request.url)
method = request.method

headers = dict(request.headers)
# User-agent header is automatically set by the browser.
# More importantly, setting it causes an error on FireFox where a preflight request is made and it leads to a CORS error.
# Maybe related to https://bugzilla.mozilla.org/show_bug.cgi?id=1629921
del headers["user-agent"]

body = None if method in ["GET", "HEAD"] else await request.aread()
response = await pyodide.http.pyfetch(
url, method=method, headers=headers, body=body
Expand Down
4 changes: 3 additions & 1 deletion js/lite/src/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,14 @@ export async function wasm_proxied_fetch(
headers[key] = value;
});

const body = request.body ?? new Uint8Array(await request.arrayBuffer()); // request.body can't be read on FireFox, so fallback to arrayBuffer().

const response = await workerProxy.httpRequest({
path: url.pathname,
query_string: url.searchParams.toString(), // The `query_string` field in the ASGI spec must not contain the leading `?`.
method,
headers,
body: request.body
body
});
return new Response(response.body, {
status: response.status,
Expand Down
14 changes: 8 additions & 6 deletions js/spa/test/file_component_events.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@ async function error_modal_showed(page) {
test("File component properly dispatches load event for the single file case.", async ({
page
}) => {
await page
.getByRole("button", { name: "Drop File Here - or - Click to Upload" })
.first()
.click();
const uploader = await page.locator("input[type=file]").first();
await uploader.setInputFiles(["./test/files/cheetah1.jpg"]);
const [fileChooser] = await Promise.all([
page.waitForEvent("filechooser"),
page
.getByRole("button", { name: "Drop File Here - or - Click to Upload" })
.first()
.click()
]);
await fileChooser.setFiles(["./test/files/cheetah1.jpg"]);

await expect(page.getByLabel("# Load Upload Single File")).toHaveValue("1");

Expand Down
14 changes: 8 additions & 6 deletions js/spa/test/gallery_component_events.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,14 @@ test("Gallery select event returns the right value and the download button works
test("Gallery click-to-upload, upload and change events work correctly", async ({
page
}) => {
await page
.getByRole("button", { name: "Drop Media Here - or - Click to Upload" })
.first()
.click();
const uploader = await page.locator("input[type=file]").first();
await uploader.setInputFiles([
const [fileChooser] = await Promise.all([
page.waitForEvent("filechooser"),
page
.getByRole("button", { name: "Drop Media Here - or - Click to Upload" })
.first()
.click()
]);
await fileChooser.setFiles([
"./test/files/cheetah1.jpg",
"./test/files/cheetah1.jpg"
]);
Expand Down
32 changes: 24 additions & 8 deletions js/spa/test/outbreak_forecast.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { test, expect } from "@self/tootils";
import { test, expect, is_lite } from "@self/tootils";

test("selecting matplotlib should show matplotlib image and pressing clear should clear output", async ({
page
Expand All @@ -20,18 +20,27 @@ test("selecting matplotlib should show matplotlib image and pressing clear shoul
});

test("selecting plotly should show plotly plot and pressing clear should clear output", async ({
page
page,
browserName
}) => {
test.fixme(
browserName === "firefox" && is_lite,
"Plotly component can't be located on Lite on FireFox in the CI env for some reason"
);

await page.getByLabel("Plot Type").click();
await page.getByRole("option", { name: "Plotly" }).click();
await page.getByLabel("Month").click();
await page.getByRole("option", { name: "January" }).click();
await page.getByLabel("Social Distancing?").check();

await page.click("text=Submit");
await expect(page.locator(".js-plotly-plot")).toHaveCount(1);

const plotly_plot = page.getByTestId("plotly");
await expect(plotly_plot).toHaveCount(1);

await page.getByRole("button", { name: "Clear" }).click();
await expect(page.locator(".js-plotly-plot")).toHaveCount(0);
await expect(plotly_plot).toHaveCount(0);
});

test("selecting altair should show altair plot and pressing clear should clear output", async ({
Expand All @@ -45,7 +54,7 @@ test("selecting altair should show altair plot and pressing clear should clear o

await page.click("text=Submit");

const altair = await page.getByTestId("altair");
const altair = page.getByTestId("altair");
await expect(altair).toHaveCount(1);

await page.getByRole("button", { name: "Clear" }).click();
Expand All @@ -63,16 +72,22 @@ test("selecting bokeh should show bokeh plot and pressing clear should clear out

await page.click("text=Submit");

const altair = await page.getByTestId("bokeh");
const altair = page.getByTestId("bokeh");
await expect(altair).toHaveCount(1);

await page.getByRole("button", { name: "Clear" }).click();
await expect(altair).toHaveCount(0);
});

test("switching between all 4 plot types and pressing submit should update output component to corresponding plot type", async ({
page
page,
browserName
}) => {
test.fixme(
browserName === "firefox" && is_lite,
"Plotly component can't be located on Lite on FireFox in the CI env for some reason"
);

//Matplotlib
await page.getByLabel("Plot Type").click();
await page.getByRole("option", { name: "Matplotlib" }).click();
Expand All @@ -91,7 +106,8 @@ test("switching between all 4 plot types and pressing submit should update outpu
await page.getByRole("option", { name: "Plotly" }).click();

await page.click("text=Submit");
await expect(page.locator(".js-plotly-plot")).toHaveCount(1);
const plotly = page.getByTestId("plotly");
await expect(plotly).toHaveCount(1);

//Altair
await page.getByLabel("Plot Type").click();
Expand Down
2 changes: 1 addition & 1 deletion js/tootils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const ROOT_DIR = path.resolve(
"../../../.."
);

const is_lite = !!process.env.GRADIO_E2E_TEST_LITE;
export const is_lite = !!process.env.GRADIO_E2E_TEST_LITE;

const test_normal = base.extend<{ setup: void }>({
setup: [
Expand Down
18 changes: 15 additions & 3 deletions js/wasm/src/cross-origin-worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,32 @@ function get_blob_url(url: URL): string {
return worker_blob_url;
}

function is_same_origin(url: URL): boolean {
return url.origin === window.location.origin;
}

export class CrossOriginWorkerMaker {
public readonly worker: Worker | SharedWorker;

constructor(url: URL, options?: WorkerOptions & { shared?: boolean }) {
const { shared = false, ...workerOptions } = options ?? {};

try {
if (is_same_origin(url)) {
console.debug(
`Loading a worker script from the same origin: ${url.toString()}.`
);
// This is the normal way to load a worker script, which is the best straightforward if possible.
this.worker = shared
? new SharedWorker(url, workerOptions)
: new Worker(url, workerOptions);
} catch (e) {

// NOTE: We use here `if-else` checking the origin instead of `try-catch`
// because the `try-catch` approach doesn't work on some browsers like FireFox.
// In the cross-origin case, FireFox throws a SecurityError asynchronously after the worker is created,
// so we can't catch the error synchronously.
} else {
console.debug(
`Failed to load a worker script from ${url.toString()}. Trying to load a cross-origin worker...`
`Loading a worker script from a different origin: ${url.toString()}.`
);
const worker_blob_url = get_blob_url(url);
this.worker = shared
Expand Down

0 comments on commit 9004b11

Please sign in to comment.