Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Pageactions this._handlers is null

RESOLVED FIXED in Firefox 11

Status

Fennec Graveyard
Extension Compatibility
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Axel Nennker, Assigned: mbrubeck)

Tracking

({regression, verified-beta})

Firefox 9
Firefox 11
ARM
Android
regression, verified-beta

Details

Attachments

(3 attachments)

(Reporter)

Description

6 years ago
Created attachment 573870 [details]
openid.xpi more debug output than the AMO version

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:8.0) Gecko/20100101 Firefox/8.0
Build ID: 20111104165243

Steps to reproduce:

Install OpenID for Firefox (attached) in Firefox Beta on Android and clicked the openid icon in the pageactions. 
https://addons.mozilla.org/en-US/firefox/addon/openid-for-firefox/
This happens in Firefox Beta (Gecko/20111109 Firefox/9.0).
This works in the current Firefox Mobile (Gecko/20111104) on Android.


Actual results:

Nothing. Pageactions closed because PageActions.Register threw this._handlers is null.


Expected results:

The user's openid should have been entered into the openid_url input field.

The console output shows the error message "TypeError: this._handlers is null" in the function that calls PageActions.register.
(Reporter)

Updated

6 years ago
OS: Windows 7 → Android
Hardware: x86_64 → ARM

Updated

6 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 1

6 years ago
Gecko/20111114 Firefox/11.0a1 has this bug also.
An xpi to test for yourself is here: version 1.2.10
https://addons.mozilla.org/en-US/firefox/addon/openid-for-firefox/versions/
The version 1.2.10 works with the current release of Firefox and only one feature (PageAction) of the addon does not work in Beta and Nighly.

Comment 2

6 years ago
I got the same issue.  Find this one before open a new bug.

In fennec 10.x, such as fennec-10.0a2, the PageActions.register function does not work. In try/catch, It always report "TypeError: this._handlers is null".   Please support this issue.

In my other test, PageActions.register does work at fennec 8.x such as fennec-8.0a1
(Assignee)

Updated

6 years ago
Assignee: nobody → mbrubeck
status-firefox10: --- → affected
status-firefox11: --- → affected
status-firefox9: --- → affected
Keywords: regression
(Assignee)

Comment 3

6 years ago
This is a regression from bug 687729.
Blocks: 687729
Status: NEW → ASSIGNED
status-firefox8: --- → unaffected
(Assignee)

Comment 4

6 years ago
Created attachment 580977 [details] [diff] [review]
patch

A simple one-line fix.  We need this on Aurora and Beta too.
Attachment #580977 - Flags: review?(mark.finkle)
Attachment #580977 - Flags: review?(mark.finkle) → review+
(Assignee)

Comment 5

6 years ago
https://hg.mozilla.org/mozilla-central/rev/753ed712f1be
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
status-firefox11: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 11
(Assignee)

Comment 6

6 years ago
Comment on attachment 580977 [details] [diff] [review]
patch

Requesting approval for Aurora 10 and Beta 9.  This is a one-line mobile-only fix for a regression in update 9 that breaks some extensions.  The fix is very safe and has little to no potential for regressions or perf impact.

Note: The alternate, less-desirable solution would be for the author of each affected add-on to add "if (PageActions._handlers === null) PageActions.init();" before their first PageActions.register call, until a fixed version of Firefox is released.
Attachment #580977 - Flags: approval-mozilla-beta?
Attachment #580977 - Flags: approval-mozilla-aurora?

Comment 7

6 years ago
Comment on attachment 580977 [details] [diff] [review]
patch

[Triage Comment]
Let's take this low-risk mobile-only fix on aurora/beta. Please land asap to make it into our final beta.
Attachment #580977 - Flags: approval-mozilla-beta?
Attachment #580977 - Flags: approval-mozilla-beta+
Attachment #580977 - Flags: approval-mozilla-aurora?
Attachment #580977 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 8

6 years ago
https://hg.mozilla.org/releases/mozilla-beta/rev/f9fedb00497d
https://hg.mozilla.org/releases/mozilla-aurora/rev/04043d9a960f
status-firefox10: affected → fixed
status-firefox9: affected → fixed
(Reporter)

Comment 9

6 years ago
Created attachment 581222 [details]
Test Case

Thank you for fixing this. Although I don't neither what's wrong with the previous code in common-ui.js not why the solution should work.

I copied the definition of PageActions from http://mxr.mozilla.org/mobile-browser/source/chrome/content/common-ui.js and put it into the attached file to have an easy test. If you load PageActions.html into Firefox (Desktop) then the output is "aId" and this is what I expect.
This test does not test this bug nor your solution.

Could you please explain why it is even possible that _handlers can not be initialized? Calling PageActions.init does not set _handlers either. Why does your fix fix this problem?

Thanks.
(Reporter)

Comment 10

6 years ago
Answering my own question; Sorry.
I looked at the wrong file. The correct file is
https://hg.mozilla.org/releases/mozilla-beta/file/f9fedb00497d/mobile/chrome/content/PageActions.js
not 
http://mxr.mozilla.org/mobile-browser/source/chrome/content/common-ui.js

common-ui.js initialized _handlers like so:
var PageActions = {
  _handlers : [],
};

while PageActions.js does initialize _handlers in init:
var PageActions = {
  _handlers : null,
  init : function() {
    if (this._handlers) return;
    this._handlers = [];
  }
};

Isn't the 'usual' Mozilla coding pattern to define a getter for things like _handlers and initialize them on first use?
Verified fixed on Firefox9 Beta6.
Mozilla /5.0 (Android;Linux armv7l;rv:9.0) Gecko/20111212 Firefox/9.0 Fennec/9.0 
Device: HTC Desire (Android 2.2)
status-firefox9: fixed → ---
Keywords: verified-beta
status-firefox9: --- → fixed
(Assignee)

Comment 12

6 years ago
(In reply to Axel Nennker from comment #10)
> Isn't the 'usual' Mozilla coding pattern to define a getter for things like
> _handlers and initialize them on first use?

In this case, _handlers is serving a second purpose as a flag to tell whether init() has already been called.  This was probably a bad idea; an explicit flag would have been better and would have avoided this bug.  (I would normally submit a patch to clean this up, but we're trying to minimize changes to XUL fennec while our QA and development resources are focused on native Android fennec.)
You need to log in before you can comment on or make changes to this bug.