1426561 - TypeError: this._activeItems.elementWeakRef is undefined in FormAutofillContent.jsm when enter in urlbar

Status: VERIFIED FIXED

(Toolkit :: Form Autofill, defect)

Description

[magicp]

Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:59.0) Gecko/20100101 Firefox/59.0 ID:20171220121833

Steps to reproduce:
1. Launch the latest Nightly (20171220121833)
2. Enter in urlbar

Actual results:
The following error is logged in the browser console.

TypeError: this._activeItems.elementWeakRef is undefined FormAutofillContent.jsm:490:5

Expected results:
No error

Regression range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=deb775eb35bb3e157e89646ede5416b5f00cf8ee&tochange=18c3ccc735368550df23936bf766627a1fcba487

Comment 1

[Sean Lee [:seanlee][:weilonge]]

Attachment: Bug 1426561 - Fix the null pointer issue caused by FormAutofillContent.

Review commit: https://reviewboard.mozilla.org/r/209442/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/209442/

Comment 2

[Sean Lee [:seanlee][:weilonge]]

This patch fixes the two null pointer issues:
1. TypeError: this._activeItems.elementWeakRef is undefined in FormAutofillContent.jsm
 - when press enter key in urlbar

2. JavaScript error: resource://gre/modules/FormLikeFactory.jsm, line 109: TypeError: aField is null [1]
 - when focus on a field which is not marked.

[1] https://searchfox.org/mozilla-central/rev/d99de806665d227ad71262933800ef6b069b230a/browser/extensions/formautofill/FormAutofillContent.jsm#449

Comment 3

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8938988 [details]
Bug 1426561 - Fix the null pointer issue caused by FormAutofillContent.

https://reviewboard.mozilla.org/r/209442/#review215164

::: browser/extensions/formautofill/FormAutofillContent.jsm:479
(Diff revision 1)
>      element = element || formFillController.focusedInput;
>      let handler = this._getFormHandler(element);
>      if (handler) {
>        handler.focusedInput = element;
>      }
>      this._activeItems = {
>        handler,
> -      elementWeakRef: Cu.getWeakReference(element),
> +      elementWeakRef: element ? Cu.getWeakReference(element) : null,
>        section: handler ? handler.activeSection : null,
>        fieldDetail: null,
>      };

How about early-returning an empty object (as its initial value) if the element doesn't exist? (as I don't think we'll benefit by keeping those `null` properties in this case.) e.g.

```js
if (!element) {
  this._activeItems = {};
  return;
}
```

Comment 4

[Sean Lee [:seanlee][:weilonge]]

Comment on attachment 8938988 [details]
Bug 1426561 - Fix the null pointer issue caused by FormAutofillContent.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/209442/diff/1-2/

Comment 5

[Sean Lee [:seanlee][:weilonge]]

try link for the first patch:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c086bb871e4247d3cb0d0b3211d1d294b4c24410

Comment 6

[Sean Lee [:seanlee][:weilonge]]

Comment on attachment 8938988 [details]
Bug 1426561 - Fix the null pointer issue caused by FormAutofillContent.

https://reviewboard.mozilla.org/r/209442/#review215164

> How about early-returning an empty object (as its initial value) if the element doesn't exist? (as I don't think we'll benefit by keeping those `null` properties in this case.) e.g.
> 
> ```js
> if (!element) {
>   this._activeItems = {};
>   return;
> }
> ```

Sounds good! Updated in the latest patch.

Comment 7

[Luke Chang [:lchang] (inactive)]

Comment on attachment 8938988 [details]
Bug 1426561 - Fix the null pointer issue caused by FormAutofillContent.

https://reviewboard.mozilla.org/r/209442/#review215168

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/325e9306bf09
Fix the null pointer issue caused by FormAutofillContent. r=lchang

Comment 10

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/325e9306bf09

Comment 11

[magicp]

This bug fix has verified in the latest Nightly build (20171229220051). Thanks!