-
Notifications
You must be signed in to change notification settings - Fork 146
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
Firefox support? #445
Comments
@mike-engel - Currently Accessibility Insights for Web only supports Chrome. We haven't explored Firefox support. |
@flyingUnderTheRadar Are there any plans to support other browsers in the future? |
@mike-engel - At this point of time there is no plan to support Firefox or any other browser in the near future. |
Need to update docs to reflect this so leaving it open. |
Hey @mike-engel thanks for your interest in Accessibility Insights for Web! After long conversations with the team we have determined that for now, the costs to create an extension for Firefox would be too significant for the team. We agree that there is value in your suggestion however, for now, we want to focus on continuing providing a great experience in our current extension and improving the capabilities in it. Thanks! |
Firefox DevTools team member here, who would be available for help. Accessibility audits are important to keep the web open and accessible to all; and developers struggle with understanding and addressing this topic. Your audit has some great ideas and can have a meaningful impact! Having an product labeled "for Web" only available in Chrome would be an sad outcome for an announcement that promised:
Many developer extensions are available for Chrome and Firefox, and you won't find many CSS or JS hacks. Once setup, the builds are not much overhead. What you will end up with is cross-browser compatible and more future-proof code. We are happy to support the work to add support and the outreach necessary to get developers on pre-release channels for testing. Please re-consider and let me know how we can help. |
Hey @digitarald thank you for reaching out and offering to help! |
Great to hear @adrianaandreeva! Looking forward to hear the results of the internal conversations. If we know more about what is blocking your work, we can diagnose and come up with estimates on by when it should be fixed. Feel free to reach out via |
Please add Firefox support! With WebExtensions the APIs are very much the same and your maintenance cost will be very low. (actually the Firefox WebExtension APIs are often better and have more features.) Also please re-open this issue, so we can see you are considering adding Firefox support. |
Looked into this a little during slack time last week. #1003 #1004 #1005 (and Firefox 1570849) came out of it, but only got as far as enabling the popup page to show up; the ad-hoc tools hang when being enabled and the fastpass/assessments links don't launch the desired pages, and the background page is full of console errors about unhandled errors in Notes on P2 things so we don't forget about them if/when we revisit this:
|
Okay, so the only drawback would be that |
A few more implementation notes:
|
#### Description of changes Firefox's `RegExp` implementation doesn't support named capture groups ([bugzilla 1362154](https://bugzilla.mozilla.org/show_bug.cgi?id=1362154)). This updates the one place we currently use them to use an index-based capture group instead. No impact on Chromium. #### Pull request checklist <!-- If a checklist item is not applicable to this change, write "n/a" in the checkbox --> - [x] Addresses an existing issue: part of #445 - [x] Ran `yarn fastpass` - [n/a (existing test already covers implementation)] Added/updated relevant unit test(s) (and ran `yarn test`) - [x] Verified code coverage for the changes made. Check coverage report at: `<rootDir>/test-results/unit/coverage` - [x] PR title *AND* final merge commit title both start with a semantic tag (`fix:`, `chore:`, `feat(feature-name):`, `refactor:`). See `CONTRIBUTING.md`. - [n/a] (UI changes only) Added screenshots/GIFs to description above - [n/a] (UI changes only) Verified usability with NVDA/JAWS
…ate() (#2746) #### Description of changes Firefox and Chromium have different behavior when the webextension `windows.create()` API is invoked with a relative URL; Chromium resolves the relative URL relative to the root of the webextension, where Firefox resolves it relative to the path containing the invoking page. In practice, this means that Firefox tries to open DetailsView/debug pages as `moz-extension://extensionid/background/debug-tools/debug-tools.html` (note the extra `/background/` in there). This resolves the firefox issue without impacting the chromium behavior by updating the cases that use relative URLs to instead use document-root-relative URLs, which both engines resolve as relative to the extension origin. The combination of this, #2744, and #2745, and using Firefox 77 (beta) is enough to get a details view to load and mostly render in Firefox. There are still further compatibility issues out of scope for this PR (the menus in the popup view don't work, there are a few minor rendering issues in the requirements list, and all interactions with the target page seem broken) #### Pull request checklist <!-- If a checklist item is not applicable to this change, write "n/a" in the checkbox --> - [x] Addresses an existing issue: progress on #445 - [x] Ran `yarn fastpass` - [x] Added/updated relevant unit test(s) (and ran `yarn test`) - [x] Verified code coverage for the changes made. Check coverage report at: `<rootDir>/test-results/unit/coverage` - [x] PR title *AND* final merge commit title both start with a semantic tag (`fix:`, `chore:`, `feat(feature-name):`, `refactor:`). See `CONTRIBUTING.md`. - [n/a] (UI changes only) Added screenshots/GIFs to description above - [n/a] (UI changes only) Verified usability with NVDA/JAWS
… Firefox (#2744) #### Description of changes This PR moves `BrowserAdapter` creation into a dedicated `BrowserAdapterFactory` class to facilitate using a different adapter type for different browsers. It creates the beginning of a separate `FirefoxAdapter` vs `ChromiumAdapter` with the differences between them in place that we know about already; this is enough to allow Firefox 77 Beta to open a details view page (it doesn't render anything yet, but it at least stops the error you get when trying to open it at all). #### Pull request checklist <!-- If a checklist item is not applicable to this change, write "n/a" in the checkbox --> - [x] Addresses an existing issue: part of #445 - [x] Ran `yarn fastpass` - [x] Added/updated relevant unit test(s) (and ran `yarn test`) - [x] Verified code coverage for the changes made. Check coverage report at: `<rootDir>/test-results/unit/coverage` - [x] PR title *AND* final merge commit title both start with a semantic tag (`fix:`, `chore:`, `feat(feature-name):`, `refactor:`). See `CONTRIBUTING.md`. - [n/a] (UI changes only) Added screenshots/GIFs to description above - [n/a] (UI changes only) Verified usability with NVDA/JAWS
Hi @dbjorge, I'm excited you are looking at Firefox support, even if your team has not prioritized it. For the second item on your list, we're looking into a quick fix that would avoid I hope this makes Firefox development easier, if you have any additional feedback please feel free to get in touch! |
@kewisch Thanks for the update! We're actually the case where we pass For other feedback: the incompatibility that I spent today tracking down was the difference in behavior between how Firefox and Chromium interpret relative paths passed in the I understand that there isn't a great way to fix the behavior difference without it being a breaking change, but it would be helpful if the error message could identify the issue more concretely (bonus points for specifically detecting a relative path as the file parameter and linking the documentation about that gotcha on MDN as part of the error message) |
…3172) #### Description of changes This PR fixes the largest remaining blocker for Firefox support; it enables ai-web to successfully inject code into the target page. With this PR, the extension is mostly-functional in Firefox, though there are still several known issues outstanding that it does not address (see #445). ![screenshot of injected automated checks dialog rendering on a target page in Firefox](https://user-images.githubusercontent.com/376284/88836482-c220e980-d1a4-11ea-8319-5eefef786fd6.png) The root cause of the incompatibility was this note buried in the documentation for the file parameter of the [`browser.tabs.insertCSS`](https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/tabs/insertCSS) and [`browser.tabs.executeScript`](https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/tabs/executeScript) APIs: > **`file`** | *Optional* > `string`. Path to a file containing the code to inject. In Firefox, relative URLs are resolved relative to the current page URL. In Chrome, these URLs are resolved relative to the extension's base URL. To work cross-browser, you can specify the path as an absolute URL, starting at the extension's root, like this: `"/path/to/stylesheet.css"`. This issue took a while to track down because the error message you get from Firefox when you ask it to inject a CSS/JS file with an invalid file path contains no useful information; it just says `Error: an unexpected error occurred`. We happen to use it here in a context where we're using `.then`/`.catch` callback handling all the way through our call stack, rather than `async`/`await`, which further exacerbated debugging due to not having full stack information from the error. Technically, the only updates strictly required here were adding the `/` prefixes to the `cssFiles` and `jsFiles` in `content-script-injector.ts` and to the link elements in `shadow-initializer.ts`. The remaining changes are: * Updates to `ContentScriptInjector` to log errors during injection to `logger.error` with context about which tab/file had trouble * Updates to `WebextensionBrowserAdapter` to fail-fast with a descriptive error if a relative path would be passed to `insertCSS`/`executeScript` #### Pull request checklist <!-- If a checklist item is not applicable to this change, write "n/a" in the checkbox --> - [x] Addresses an existing issue: Part of #445 - [x] Ran `yarn fastpass` - [x] Added/updated relevant unit test(s) (and ran `yarn test`) - [x] Verified code coverage for the changes made. Check coverage report at: `<rootDir>/test-results/unit/coverage` - [x] PR title *AND* final merge commit title both start with a semantic tag (`fix:`, `chore:`, `feat(feature-name):`, `refactor:`). See `CONTRIBUTING.md`. - [x] (UI changes only) Added screenshots/GIFs to description above - [x] (UI changes only) Verified usability with NVDA/JAWS
#### Description of changes The outcome summary bar was using `flex-basis: 100%`, which is intended to imply that child elements should use the height of the containing element by default. This is wrong; we don't want the summary bar to be the height of the whole overview panel. Firefox was doing the right thing (by respecting this incorrect `flex-basis` value), where Chromium for some reason ignores the bogus value. No visual impact in Chromium. In Firefox, before (all the content is there if you scroll way down): ![screenshot showing buggy spacing in firefox](https://user-images.githubusercontent.com/376284/88963922-8f472600-d276-11ea-9905-daf0ae7a1f90.png) after: ![screenshot showing overview page rendering as-designed in firefox](https://user-images.githubusercontent.com/376284/88963861-7c345600-d276-11ea-86e2-4e58b002c103.png) #### Pull request checklist <!-- If a checklist item is not applicable to this change, write "n/a" in the checkbox --> - [x] Addresses an existing issue: part of #445 - [x] Ran `yarn fastpass` - [n/a] Added/updated relevant unit test(s) (and ran `yarn test`) - [n/a] Verified code coverage for the changes made. Check coverage report at: `<rootDir>/test-results/unit/coverage` - [x] PR title *AND* final merge commit title both start with a semantic tag (`fix:`, `chore:`, `feat(feature-name):`, `refactor:`). See `CONTRIBUTING.md`. - [x] (UI changes only) Added screenshots/GIFs to description above - [n/a] (UI changes only) Verified usability with NVDA/JAWS
…ows (#3177) #### Description of changes Previously, in Firefox, selecting "Settings" or "Preview Features" from the gear menu in the details page would always launch an extra new details view window, in addition to opening the panel it's supposed to. The root cause is that the actions that handle those menu item presses are the same actions that handle the similar item presses in the popup page's gear menu; they choose whether to launch a new details view window or not as part of responding to the request by checking whether the ExtensionDetailsViewController already knows about a details view tab being open for a given target tab ID, and opening a new details view tab if not. However, the tracking mechanism the control uses for detecting those details view tabs being open is based on looking for tab-update events with URL properties that match the output of this function: ```typescript private getDetailsUrlWithExtensionId(tabId: number): string { return `${this.browserAdapter.getRunTimeId()}${this.getDetailsUrl(tabId)}`; } ``` This works in Chrome because the extension URL prefix is based on the runtime ID. However, in Firefox, the runtime ID and the extension URL prefix are not necessarily related to one another (in particular, they are completely distinct for unpacked extensions), so the controller never "notices" that a details view tab was opened; this means that it thinks it has to launch a new one every time. This PR updates the logic to use `getUrl(...)` instead of trying to format the URL manually using `getRunTimeId()`. Since this was the only point where we actually used `getRunTimeId()`, I removed it from the `BrowserAdapter` interface. #### Pull request checklist <!-- If a checklist item is not applicable to this change, write "n/a" in the checkbox --> - [x] Addresses an existing issue: part of #445 - [x] Ran `yarn fastpass` - [x] Added/updated relevant unit test(s) (and ran `yarn test`) - [x] Verified code coverage for the changes made. Check coverage report at: `<rootDir>/test-results/unit/coverage` - [x] PR title *AND* final merge commit title both start with a semantic tag (`fix:`, `chore:`, `feat(feature-name):`, `refactor:`). See `CONTRIBUTING.md`. - [n/a] (UI changes only) Added screenshots/GIFs to description above - [n/a] (UI changes only) Verified usability with NVDA/JAWS
#### Description of changes This PR is an experiment to try migrating our web e2e tests from Puppeteer to Playwright. We'll probably want to discuss it in Engineering Discussion before merging. [Playwright](https://playwright.dev/) is a relatively new (released 1.0 in January) competitor to Selenium/Puppeteer/Cypress, maintained by Microsoft and developed by a team that includes several ex-Puppeteer team members. Its API surface is very similar to Puppeteer's (note that there are very few changes to individual tests required in this migration PR), but it offers several advantages that are relevant to our project compared to Puppeteer: * Playwright's selectors pierce the shadow DOM by default (we needed to do some tricky workarounds for this with Puppeteer to test our target page visualizations) * Playwright's selectors are generally more flexible; they support xpath based queries, innertext-based queries, and even innertext+regex queries. The latter two require more effort with Puppeteer. * Playwright took a page out of Cypress's book and will, by default, wait for elements to be ["actionable"](https://playwright.dev/#version=v1.2.1&path=docs%2Factionability.md&q=) as a part of all its APIs which interact with elements on a page. This is a *big deal* for test reliability, and would allow us to obsolete a lot of our `Page` helpers that try to do similar actionability checks (but in a less complete/robust way than Playwright advertises). * Playwright's error messages are dramatically more actionable than Puppeteer's. For example, when I was doing this migration PR, there were a few tests in `visualization-boxes.test.ts` that were failing when they tried to look for the `#insights-shadow-host .insights-highlight-container` selector. In Puppeteer, this would have shown up in Jest logs as something like "Element not found", and it would have been up to you to figure out which element based on the stack. In Playwright, you still have the same stack info, but you additionally get this message which explains not only which element wasn't found, but context about *why* it wasn't found (in this case, that it was in the DOM but not yet visible as of the timeout): ``` TimeoutError: Timeout 5000ms exceeded during page.waitForSelector. ================ page.waitForSelector logs ================ [api] waiting for selector "#insights-shadow-host #insights-shadow-container" to be visible [api] selector resolved to hidden <div id="insights-shadow-container">…</div> ============================================================ Note: use DEBUG=pw:api environment variable and rerun to capture Playwright logs. ``` * Playwright's Firefox support is more complete (it is mostly-supported, but extension support requires workarounds; see microsoft/playwright#2644 and microsoft/playwright#2874 - Puppeteer's support for Firefox is currently experimental and much less complete) * Playwright maintains their own headful-browser-compatible docker base image, so we don't have to rely on Cypress's even though we aren't using Cypress * Playwright is written in Typescript, so its typings are included with the main npm package and cannot become out of date like `@types/puppeteer` #### Pull request checklist <!-- If a checklist item is not applicable to this change, write "n/a" in the checkbox --> - [x] Addresses an existing issue: part of enabling e2e tests for #445 - [x] Ran `yarn fastpass` - [n/a] Added/updated relevant unit test(s) (and ran `yarn test`) - [n/a] Verified code coverage for the changes made. Check coverage report at: `<rootDir>/test-results/unit/coverage` - [x] PR title *AND* final merge commit title both start with a semantic tag (`fix:`, `chore:`, `feat(feature-name):`, `refactor:`). See `CONTRIBUTING.md`. - [n/a] (UI changes only) Added screenshots/GIFs to description above - [n/a] (UI changes only) Verified usability with NVDA/JAWS
Thank you for the feedback Dan. I've filed bug 1661125 on our end to improve the error messages. We'd love to increase compatibility between browsers and will see how to best approach this for the future. Let us know if you come across any other differences that were difficult to debug. |
Why is this issue closed? I would love to use this amazing tool in Firefox. |
Hey @shawnthompson thanks for your question and your interest in Accessibility Insights! We revisited this issue internally and we still cannot prioritize this work. As we commented back when this issue was created, the cost to create and maintain this extension for Firefox would be too impactful for the team. |
Thanks @ferBonnin. What about having it completely open and let the community support this? Mozilla has a great team of developers that who worked very hard to get Firefox to work with VoiceOver. VoiceOver Preview for macOS Firefox Maybe they have some bandwidth to work with you folks on this. I'm actually demoing A11Y Insights to my team later this morning and will be speaking about it to a larger group within the Government of Canada. Many departments within the "system" do not allow Chrome but they do allow Firefox within the department. I'm sure I'm not alone in not wanting to start another browser war. Side note: let's not forget about Safari!!! |
After reviewing within our team and hearing the community feedback, we are reactivating this feature request. Our original concerns around the initial and ongoing cost of maintaining support for Firefox are still valid:
but we would be happy to accept ongoing community contributions that enable the extension to work properly in Firefox with every release, and that update our E2E tests to cover Firefox scenarios. |
Thanks Fer! To summarize from earlier comments, the known work that we'd love to see community contributions for includes (but isn't limited to):
|
Unfortunately, this is out of scope given our current priorities. Thanks for using Accessibility Insights! |
Hey y'all! I'm sorry if this is mentioned somewhere, but I couldn't find anything about support for other browsers (specifically firefox for me).
I know that the firefox addon api is similar to chrome, but I unfortunately don't know the specifics about what's different.
Thanks!
The text was updated successfully, but these errors were encountered: