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

Don't use deprecated initKeyEvent() for autofill #2001

Merged
merged 1 commit into from
Aug 11, 2021
Merged

Don't use deprecated initKeyEvent() for autofill #2001

merged 1 commit into from
Aug 11, 2021

Conversation

mathewhodson
Copy link
Contributor

This fixes #2000

@CLAassistant
Copy link

CLAassistant commented Aug 11, 2021

CLA assistant check
All committers have signed the CLA.

@jonathonf
Copy link

jonathonf commented Aug 11, 2021

Can confirm this is working with 93.0a1 20210810213316

@streetwolf
Copy link

Can I download the version of BW that contains this patch. I use Nightly and really need this.

@MGibson1 MGibson1 requested a review from a team August 11, 2021 12:51
@jonathonf
Copy link

jonathonf commented Aug 11, 2021

@streetwolf I can send you my unsigned XPI that you can install via about:debugging, though that would be very much an "at your own risk" situation (i.e. how much you can trust a random person on the Web not to steal your passwords versus how difficult it would be to build it yourself).

Edit: Use the official build artefacts instead, #2001 (comment)

@streetwolf
Copy link

streetwolf commented Aug 11, 2021 via email

Copy link
Member

@MGibson1 MGibson1 left a comment

Choose a reason for hiding this comment

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

This looks good to me. Thank you, it looks like this has been teetering on the edge of support for a while now.

@cscharf @bitwarden/dept-qa I don't think this is critical to get into this release since the nightly just dropped support (FF nightlies are usually a few months out from stable). However, if you think there's capacity for adding this to the rc branches cut last night it's probably a good addition.

@MGibson1
Copy link
Member

Once this is merged into master it should also create build artifacts that can be installed in the same location.

@jonathonf
Copy link

jonathonf commented Aug 11, 2021

@streetwolf I edited in the link in my previous post (notification emails won't show those edits...) so here it is again. :)

Edit: Use the official build artefacts instead, #2001 (comment)

@streetwolf
Copy link

streetwolf commented Aug 11, 2021 via email

@cscharf
Copy link
Contributor

cscharf commented Aug 11, 2021

@MGibson1 , let's please ensure we have the linked Github issue in Asana and is queued up for testing, but not in rc, we can merge this into master and leave it for the next release. The QA team has started regression testing already for the upcoming release.

@denschub
Copy link

@MGibson1 just a heads up:

FF nightlies are usually a few months out from stable

Nightly 93, which breaks Bitwarden, will go into Beta on September 6 and most likely be release on October 5. So that's ~8 weeks.

@cscharf
Copy link
Contributor

cscharf commented Aug 11, 2021

Nightly 93, which breaks Bitwarden, will go into Beta on September 6 and most likely be release on October 5. So that's ~8 weeks.

We'll definitely be able to roll it out before then.

@streetwolf
Copy link

As already stated the unsigned version with the patch hasn't caused me any problems so far. Nice job. Hope the signed version becomes available soon.

@MGibson1 MGibson1 merged commit da12cbc into bitwarden:master Aug 11, 2021
@MGibson1
Copy link
Member

We only sign our release builds, but here is the link to the Github actions build for Firefox: https://github.com/bitwarden/browser/suites/3472198561/artifacts/82556656

@streetwolf
Copy link

streetwolf commented Aug 11, 2021 via email

@MGibson1
Copy link
Member

The master branch already contains all the code which will be included in the next release. You've already got the good stuff :)

That being said, please recognize that these non-release builds do not go through our regression testing process and many may be totally untested. Expect more issues than with an actual release build.

@Caspy7
Copy link

Caspy7 commented Aug 20, 2021

It was mentioned in another issue but not this one, there is a simpler resolution to this problem: Go to about:config in the URL bar, find dom.keyboardevent.init_key_event.enabled and toggle it to true.

Apparently the deprecated API in question has not been removed from the code yet, just disabled. By the time it is removed, release Bitwarden will [hopefully] have updated to mitigate the issue.

@BBaoVanC
Copy link

Was this included in 1.52.0? Firefox auto-updated it to the latest version and autofill stopped working again (but using the artifact mentioned here works)

@denschub
Copy link

Was this included in 1.52.0?

No, it was not: https://github.com/bitwarden/browser/blob/v1.52.0/src/content/autofill.js#L843

@bb010g
Copy link

bb010g commented Aug 21, 2021

How do merges into the rc branch work?

@eliykat
Copy link
Member

eliykat commented Aug 22, 2021

Thank you @Caspy7 for the workaround. I confirm this wasn't included in 1.52.0 but will be in the following release, per @cscharf's comment above. I don't have an exact date for that release, but it'll be before 5 October, when this breaking change is expected to be released in FF stable.

@bb010g: we develop off the master branch. 1-2 weeks before a release, we cut rc (release candidate) branches for each repo, and those branches are used for regression testing and deployment (not master). The date that we cut rc branches is the effective cut-off date for features and fixes. This issue and fix just happened to come in shortly after we'd cut rc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Firefox: Dependency on KeyboardEvent.initKeyEvent causes auto-fill breakage in Nightly