Closed Bug 1163816 Opened 9 years ago Closed 9 years ago

"modelFor is not defined" in sdk/tabs/tab-fennec.js line: 258

Categories

(Add-on SDK Graveyard :: General, defect)

defect
Not set
normal

Tracking

(firefox39 unaffected, firefox40+ fixed, firefox41+ fixed)

RESOLVED FIXED
mozilla41
Tracking Status
firefox39 --- unaffected
firefox40 + fixed
firefox41 + fixed

People

(Reporter: offbynone, Assigned: mossop)

References

Details

(Keywords: addon-compat, regression)

Attachments

(2 files)

User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:40.0) Gecko/20100101 Firefox/40.0
Build ID: 20150511030203
Firefox for Android

Steps to reproduce:

Create an addon and require('sdk/tabs');
Build it for fennec and install addon.
Try to run the code


Actual results:

addon hits error and thus fails to start
console will show ReferenceError: modelFor is not defined <unknown>
Put a breakpoint on the line requiring sdk/tabs and you will be able to get the exception with the details:
fileName: resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/tabs/tab-fennec.js
lineNumber: 258
message: modelFor is not defined
stack: "@resource://gre/modules/commonjs/sdk/tabs/tab-fennec.js:258:1CuddlefishLoader/options<.load@resource://gre/modules/commonjs/sdk/loader/cuddlefish.js:79:18@resource://gre/modules/commonjs/sdk/tabs/tab.js:15:20CuddlefishLoader/options<.load@resource://gre/modules/commonjs/sdk/loader/cuddlefish.js:79:18@resource://gre/modules/commonjs/sdk/windows/tabs-fennec.js:7:17CuddlefishLoader/options<.load@resource://gre/modules/commonjs/sdk/loader/cuddlefish.js:79:18@resource://gre/modules/commonjs/sdk/tabs.js:11:20CuddlefishLoader/options<.load@resource://gre/modules/commonjs/sdk/loader/cuddlefish.js:79:18CuddlefishLoader/options<.load@resource://gre/modules/commonjs/sdk/loader/cuddlefish.js:79:18main@resource://jid1-a3bemgzqqjovng-at-jetpack/lib/main.js:10:29run@resource://gre/modules/commonjs/sdk/addon/runner.js:151:1startup/</<@resource://gre/modules/commonjs/sdk/addon/runner.js:86:7Handler.prototype.process@resource://gre/modules/Promise-backend.js:867:23this.PromiseWalker.walkerLoop@resource://gre/modules/Promise-backend.js:746:7this.PromiseWalker.scheduleWalkerLoop/<@resource://gre/modules/Promise-backend.js:688:37Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise-backend.js:688:5this.PromiseWalker.schedulePromise@resource://gre/modules/Promise-backend.js:709:7this.PromiseWalker.completePromise@resource://gre/modules/Promise-backend.js:671:7readURI/<@resource://gre/modules/commonjs/sdk/net/url.js:55:9NetUtil_asyncFetch2/<.onStopRequest@resource://gre/modules/NetUtil.jsm:212:17"



Expected results:

should not have thrown the error.


This starting happening with yesterdays build (5-10-2015) on Nightly Firefox for android - It is on my phone that I see the error.  My tablet still has the version from Saturday (5-9-2015) and it does not produce this problem.
Could you attach a minimal self-contained add-on/testcase to reproduce the error, please. It'd help to find a possible regression range.
Flags: needinfo?(offbynone)
Keywords: testcase-wanted
The addon should open a tab to this page when installed.  It will run on desktop or mobile.  It works on my nexus 7 with the build from 5-9-2015 but not on my htc one m7 on any build since 5-9
Flags: needinfo?(offbynone)
Aurora 39 (and Nightly 40 for most of the time) branches were free from this bug. I found it in very first Aurora 40 build. Who added these faulty last lines to tab-fennec.js and WHAT FOR?
I guess
var{modelFor}=require("../model/core");
was missed in the beginning of tab-fennec.js (or may be const{...?).
Component: Untriaged → General
Flags: needinfo?(evold)
Product: Firefox → Add-on SDK
Version: 40 Branch → unspecified
[Tracking Requested - why for this release]:
Serious add-on compat issue on fennec


Pulling in some Fennec folks as well. Erik, can you investigate this in the near future? This looks pretty broken.
This seems to be a regression. Adding a tracking flag for firefox40 and firefox41 as it's also marked as affected.
Mossop worked on the file last, I'll let him take a look.  I'm not sure who else is working on the SDK at the moment.
Flags: needinfo?(evold) → needinfo?(dtownsend)
(In reply to Alexander from comment #4)
> I guess
> var{modelFor}=require("../model/core");
> was missed in the beginning of tab-fennec.js (or may be const{...?).

This is exactly right, it's fallout from bug 854982.

Clearing testcase-wanted since we have automated tests for this, we just don't run them on Fennec: bug 809143
Assignee: nobody → dtownsend
Blocks: 854982
Flags: needinfo?(dtownsend)
Attached file pull request
One other problem here, getTabForBrowser from helpers.js used to return null and now returns undefined. Not sure what else relied on that so I switched it back to null.
Attachment #8608949 - Flags: review?(evold)
Attachment #8608949 - Flags: review?(evold) → review+
https://hg.mozilla.org/mozilla-central/rev/d455e4d9198d
Status: UNCONFIRMED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Comment on attachment 8608949 [details] [review]
pull request

Approval Request Comment
[Feature/regressing bug #]: bug 854982
[User impact if declined]: Some SDK based add-ons won't work on Fennec
[Describe test coverage new/current, TreeHerder]: Only manual testing :(
[Risks and why]: Low risk, this can only improve the situation which is totally broken at this point
[String/UUID change made/needed]: None
Attachment #8608949 - Flags: approval-mozilla-aurora?
Please, excuse me if I didn't get it right, but flags show only firefox41 is fixed. Firefox40 is still "affected". Does it mean it will go beta, then release with this bug not fixed?
(In reply to Alexander from comment #15)
> Please, excuse me if I didn't get it right, but flags show only firefox41 is
> fixed. Firefox40 is still "affected". Does it mean it will go beta, then
> release with this bug not fixed?

No, that's what comment #14 is about.
Comment on attachment 8608949 [details] [review]
pull request

Should be safe, taking it.
Attachment #8608949 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.