Closed Bug 1149775 Opened 5 years ago Closed 4 years ago

(SeaMonkey) TypeError: tabbrowser.getTabForBrowser is not a function TypeError: tabbrowser.getBrowserForOuterWindowID is not a function

Categories

(SeaMonkey :: Tabbed Browser, defect, P2)

SeaMonkey 2.36 Branch
defect

Tracking

(seamonkey2.39 wontfix, seamonkey2.40 fixed, seamonkey2.41 fixed, seamonkey2.42 fixed)

RESOLVED FIXED
seamonkey2.42
Tracking Status
seamonkey2.39 --- wontfix
seamonkey2.40 --- fixed
seamonkey2.41 --- fixed
seamonkey2.42 --- fixed

People

(Reporter: kevink9876543, Assigned: philip.chee)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 6.2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/36.0.1985.67 Safari/537.36

Steps to reproduce:

SeaMonkey 2.37a1 built from c-c rev 3186083bf1dd / m-c rev 0aa30282f496
new profile

STR:
1) Install Scriptish v 0.1.12 by dragging an already downloaded xpi into the SeaMonkey window.  (Download was originally from AMO versions page.)  Restart.
2) Open a new tab
3) Type "seamonkey-project.org" (without quote) in the address bar, hit Enter to go there
4) Open the Error Console


Actual results:

The following error appears in the Error Console:

Error: TypeError: tabbrowser.getTabForBrowser is not a function
Source File: resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/tabs/utils.js
Line: 299


Expected results:

There should not be any error internal to the addon SDK.
Hmm, I can't edit the description...
This was reproduced on Ubuntu 12.04 32-bit / Linux i686, *not* what you see in that useragent string, but I'll leave the hardware/OS fields to their default of All for now.
Priority: -- → P2
Assignee: nobody → evold
I can confirm this - I'm porting Google search link fix back to Jetpack and it's working fine in SeaMonkey 2.35 but is completely broken in SeaMonkey 2.38 and 2.40a1 (Mac OS X). That extension isn't using anything beyond page-mod. Same error message as in comment #0.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Indeed, http://hg.mozilla.org/comm-central/file/5d74f3155ba2/suite/browser/tabbrowser.xml has no getTabForBrowser method. Yet http://hg.mozilla.org/mozilla-central/file/d01dd42e654b/addon-sdk/source/lib/sdk/tabs/utils.js#l305 expects one. I guess that page-mod module depends on this functionality. An obvious fix would be iterating through the tabs looking at their tab.linkedBrowser property, that would work in SeaMonkey.

This appears to be a regression from bug 1130529 (uplifted in bug 1139189).
Blocks: 1130529
Keywords: regression
Here you can have a jpm'ed testcase add-on (with a simple PageMod call inside) for SeaMonkey 2.38+. It throws the same error every time a new tab is opened.

> Error: TypeError: tabbrowser.getTabForBrowser is not a function
> Source File: resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/commonjs/sdk/tabs/utils.js
> Line: 306

It seems that the line of the error in SeaMonkey 2.38 changed to 306 (and was 299 in 2.37a1).

Maybe priority of this bug should be higher? Because it breaks every add-on that uses page-mod (including converted add-ons for firefox).
Blocks: abp
Question: this bug is not a regression in Add-on SDK, it's a regression in Seamonkey, so maybe "Product" field of this bug should be changed to "Seamonkey" instead of "Add-on SDK", or even a new bug with the correct "Product" field should be created?
I'm not creating two bugs for the same issue.  Moving to SeaMonkey.
Component: General → Tabbed Browser
Product: Add-on SDK → SeaMonkey
Version: unspecified → SeaMonkey 2.36 Branch
Duplicate of this bug: 1193238
I had this in my TODO notes:

Implement tabbrowser methods getBrowserForOuterWindowID() and getTabForBrowser()
Needed for about:performance

> Sun Sep 27 2015 21:10:38
> Error: A promise chain failed to handle a rejection. Did you forget to '.catch', or did you forget to 'return'?
> See https://developer.mozilla.org/Mozilla/JavaScript_code_modules/Promise.jsm/Promise

> Date: Sun Sep 27 2015 21:10:35 GMT+0800 (Malay Peninsula Standard Time)
> Full Message: TypeError: tabbrowser.getBrowserForOuterWindowID is not a function
> Full Stack: findTabFromWindow@chrome://global/content/aboutPerformance.js:86:24
....
Summary: (SeaMonkey) TypeError: tabbrowser.getTabForBrowser is not a function → (SeaMonkey) TypeError: tabbrowser.getTabForBrowser is not a function TypeError: tabbrowser.getBrowserForOuterWindowID is not a function
Attached patch Untested WIP patch v0.1 (obsolete) — Splinter Review
> Question: this bug is not a regression in Add-on SDK, it's a regression in
> Seamonkey,
Technically this is not a regression since we never had getTabForBrowser()
Attachment #8678601 - Flags: feedback?(kevink9876543)
(In reply to Philip Chee from comment #9)
> Technically this is not a regression since we never had getTabForBrowser()

It's a regression of Add-on SDK's PageMod implementation in Seamonkey, I guess. PageMod works in 2.35-, but in 2.36+ it doesn't work and throws that getTabForBrowser TypeError.
> It's a regression of Add-on SDK's PageMod implementation in Seamonkey, I
> guess. PageMod works in 2.35-, but in 2.36+ it doesn't work and throws that
> getTabForBrowser TypeError.

There is only one PageMod implementation. The Add-on SDK is shared code with Firefox hence identical
(In reply to Philip Chee from comment #9)
> Created attachment 8678601 [details] [diff] [review]
> Untested WIP patch v0.1
> 
> > Question: this bug is not a regression in Add-on SDK, it's a regression in
> > Seamonkey,
> Technically this is not a regression since we never had getTabForBrowser()

Would it be possible to add "parameter" (object properties): _tPos ?
e.g. https://hg.mozilla.org/mozilla-central/file/d53a52b39a95/browser/base/content/tabbrowser.xml#l1874

Thank you.
(In reply to janekptacijarabaci from comment #12)

> Would it be possible to add "parameter" (object properties): _tPos ?
> e.g.
> https://hg.mozilla.org/mozilla-central/file/d53a52b39a95/browser/base/
> content/tabbrowser.xml#l1874

It would be highly unlikely. Anything with a leading _underscore is an "internal implementation detail" and is not considered a public facing API. Firefox developers might decide to rewrite their code and remove _tPos tomorrow for all we know.
(In reply to Philip Chee from comment #13)
> (In reply to janekptacijarabaci from comment #12)
> 
> > Would it be possible to add "parameter" (object properties): _tPos ?
> > e.g.
> > https://hg.mozilla.org/mozilla-central/file/d53a52b39a95/browser/base/
> > content/tabbrowser.xml#l1874
> 
> It would be highly unlikely. Anything with a leading _underscore is an
> "internal implementation detail" and is not considered a public facing API.
> Firefox developers might decide to rewrite their code and remove _tPos
> tomorrow for all we know.

...but rather not do it. The problem is that this property uses many addons (e.g. Greasemonkey, Tab Mix Plus...).
But, of course, it's up to you...
(In reply to janekptacijarabaci from comment #14)
> ...but rather not do it. The problem is that this property uses many addons
> (e.g. Greasemonkey, Tab Mix Plus...).

Unfortunately, there is really many extensions doing this - they clearly shouldn't. Either way, on SeaMonkey they can use gBrowser.getTabIndex() which is public API (alternatively, they can iterate through gBrowser.tabs which will work on both SeaMonkey and Firefox).
Comment on attachment 8678601 [details] [diff] [review]
Untested WIP patch v0.1

This patch gets about:performance working and doesn't seem to break anything
Attachment #8678601 - Flags: review?(neil)
Comment on attachment 8678601 [details] [diff] [review]
Untested WIP patch v0.1

Personally I think this is overkill and you should base those methods on the similar _getTabForContentWindow method. We also don't set the "nodefaultsrc" attribute so that comment is entirely false.
Attachment #8678601 - Flags: review?(neil) → review-
Attachment #8678601 - Flags: feedback?(kevink9876543)
I have the same problem with my extension Direct Currency Converter that uses PageMod. It works in 2.35 but not in 2.38.
(In reply to neil@parkwaycc.co.uk from comment #17)
> Personally I think this is overkill and you should base those methods on the
> similar _getTabForContentWindow method. We also don't set the "nodefaultsrc"
> attribute so that comment is entirely false.
New patch with minimal fix as requested.
Assignee: evold → philip.chee
Attachment #8678601 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8685456 - Flags: review?(neil)
Comment on attachment 8685456 [details] [diff] [review]
Patch v1.0 minimal fix.

>-            } 
>+            }
[Ugh, well, I guess.]

>+            const tabs = this.tabs;
>+            for (var tab of tabs)
Because you've optimised the loop you've only got one reference to tabs, so you can just inline it.
Attachment #8685456 - Flags: review?(neil) → review+
Oh, and ditto for browsers of course.
Target Milestone: --- → seamonkey2.42
Comment on attachment 8685456 [details] [diff] [review]
Patch v1.0 minimal fix.

[Approval Request Comment]
Regression caused by (bug #): Some change in the Addon-SDK
User impact if declined: Extensions like Scriptish, AdBlockPlus fail to work properly. about:performance fails to work properly. etc.
Testing completed (on m-c, etc.): comm-central
Risk to taking this patch (and alternatives if risky): no risk. "regression" fix. Worst case is we are not better off than before.
String changes made by this patch: None.
Attachment #8685456 - Flags: approval-comm-beta?
Attachment #8685456 - Flags: approval-comm-aurora?
(In reply to Philip Chee from comment #22)
> User impact if declined: Extensions like Scriptish, AdBlockPlus fail to work
> properly. about:performance fails to work properly. etc.

Nope, Adblock Plus isn't currently affected by this. I added this to our tracking bug because we considered using page-mod for some new functionality. In the end, we decided to go with a different mechanism so I'll remove the dependency.
No longer blocks: abp
Pushed to comm-central on 2015-11-18
https://hg.mozilla.org/comm-central/rev/3ca63b0109a3
The error is gone in 2.42a1 Build ID: 20151210003002, but there are two others: 1231982, 1231984.
(In reply to Per Johansson from comment #25)
> The error is gone in 2.42a1 Build ID: 20151210003002, but there are two
> others: 1231982, 1231984.

Let bug numbers be clickable links: bug 1231982, bug 1231984.
> [Approval Request Comment]
> Regression caused by (bug #): Some change in the Addon-SDK
> User impact if declined: Extensions like Scriptish, AdBlockPlus fail to work
> properly. about:performance fails to work properly. etc.
> Testing completed (on m-c, etc.): comm-central
> Risk to taking this patch (and alternatives if risky): no risk. "regression"
> fix. Worst case is we are not better off than before.
> String changes made by this patch: None.
Flags: needinfo?(iann_bugzilla)
Flags: needinfo?(iann_bugzilla)
Attachment #8685456 - Flags: approval-comm-beta?
Attachment #8685456 - Flags: approval-comm-beta+
Attachment #8685456 - Flags: approval-comm-aurora?
Attachment #8685456 - Flags: approval-comm-aurora+
(In reply to Per Johansson from comment #25)
> The error is gone in 2.42a1 Build ID: 20151210003002, but there are two
> others: 1231982, 1231984.

I cannot confirm that - my Google search link fix extension (using page-mod module) is running without issues in 20151212003002 nightly. So whatever other issues there are, these are unrelated.
You need to log in before you can comment on or make changes to this bug.