1489250 - Crash when turning off a11y during auto-fill

Status: RESOLVED FIXED

(GeckoView :: General, defect, P1)

Description

[(inactive) Jim Chen [:jchen] [:darchons]]

If GV loads a page with a11y initially on, auto-fill will initialize. However, we will then crash if a11y is subsequently turned off.

Comment 1

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Bug 1489250 - Don't send window changed event if a11y is off; r?eeejay

Don't send window changed event if a11y has been turned off in the mean
time.

Comment 2

[Eitan Isaacson [:eeejay]]

Comment on attachment 9007026 [details]
Bug 1489250 - Don't send window changed event if a11y is off; r?eeejay

Eitan Isaacson [:eeejay] has approved the revision.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8a0494308f1f
Don't send window changed event if a11y is off; r=eeejay

Comment 4

[Chris Peterson [:cpeterson]]

[Tracking Requested - why for this release]:

Jim, do we need to uplift this crash fix to Fennec 62? Or does this autofill code only affect GV?

Comment 5

[(inactive) Jim Chen [:jchen] [:darchons]]

Yeah we should uplift it to Fennec 63.

Comment 6

[Chris Peterson [:cpeterson]]

[Tracking Requested - why for this release]:

This is a regression in Fennec (and GeckoView) 62.

Does this crash necessitate a 62.0.1 dot release? Or being a ride-along fix if there is a dot release for another bug?

Comment 7

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/8a0494308f1f

Comment 8

[Pascal Chevrel:pascalc]

(In reply to Jim Chen [:jchen] [:darchons] from comment #5)
> Yeah we should uplift it to Fennec 63.

Can you prepare the patch and request the uplift please? Thanks

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Bug 1330257 never got uplifted to Fennec 62, so I'm having a hard time understanding why it's marked as affected.

Comment 10

[(inactive) Jim Chen [:jchen] [:darchons]]

Yeah it should GV 62 and Fennec 63 (but not Fennec 62)

Comment 11

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 9007026 [details]
Bug 1489250 - Don't send window changed event if a11y is off; r?eeejay

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1330257
[User impact if declined]: Rare crash when using auto-fill and turning a11y on/off
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Simple crash fix
[String changes made/needed]: None

Comment 12

[Ryan VanderMeulen [:RyanVM]]

FYI, this grafts cleanly to Beta and the GV62 relbranch as-landed. Just needs approval requests.

Comment 13

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 9007026 [details]
Bug 1489250 - Don't send window changed event if a11y is off; r?eeejay

[Approval Request Comment]

If this is not a sec:{high,crit} bug, please state case for consideration: Crash fix

User impact if declined: Rare crash when using auto-fill and turning a11y on/off

Fix Landed on Version: 64

Risk to taking this patch (and alternatives if risky): No risk; simple crash fix

String or UUID changes made by this patch: None

See https://wiki.mozilla.org/Release_Management/Uplift_rules for more info.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9007026 [details]
Bug 1489250 - Don't send window changed event if a11y is off; r?eeejay

Approved for Fennec 63.0b5 and GV62.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

GECKOVIEW_62_RELBRANCH:
https://hg.mozilla.org/releases/mozilla-release/rev/ece2f6547002

Comment 16

[Chris Peterson [:cpeterson]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #9)
> Bug 1330257 never got uplifted to Fennec 62, so I'm having a hard time
> understanding why it's marked as affected.

OK. I wasn't sure if Fennec 62 was affected. (comment 4)

Comment 17

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/releases/mozilla-beta/rev/c18e69fdab06

Comment 18

[Sorina Florean [:sflorean]]

Tested with OnePlus 5T(Android 8.1.0) on 63.0b5 and we couldn't reproduce the crash following the information/steps provided. So verified as fixed.