Closed Bug 701804 Opened 8 years ago Closed 8 years ago

Pageactions this._handlers is null

Categories

(Firefox for Android Graveyard :: Extension Compatibility, defect)

Firefox 9
ARM
Android
defect
Not set

Tracking

(firefox8 unaffected, firefox9 fixed, firefox10 fixed, firefox11 fixed)

RESOLVED FIXED
Firefox 11
Tracking Status
firefox8 --- unaffected
firefox9 --- fixed
firefox10 --- fixed
firefox11 --- fixed

People

(Reporter: ignisvulpis, Assigned: mbrubeck)

References

Details

(Keywords: regression, verified-beta)

Attachments

(3 files)

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.
OS: Windows 7 → Android
Hardware: x86_64 → ARM
Status: UNCONFIRMED → NEW
Ever confirmed: true
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.
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: nobody → mbrubeck
Keywords: regression
This is a regression from bug 687729.
Blocks: 687729
Status: NEW → ASSIGNED
Attached patch patchSplinter Review
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+
https://hg.mozilla.org/mozilla-central/rev/753ed712f1be
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 11
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 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+
Attached file 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.
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)
(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.