Closed Bug 1664522 Opened 9 months ago Closed 9 months ago

tabs.captureVisibleTab() fails with "this.nativeTab.ownerGlobal is undefined"

Categories

(WebExtensions :: Android, defect, P2)

Product:
WebExtensions
Component:
Android
Type:
defect

Tracking

(firefox-esr68 unaffected, firefox-esr78 unaffected, firefox80 unaffected, firefox81 unaffected, firefox82 fixed)

Status:
RESOLVED FIXED
Milestone:
82 Branch
Tracking Flags:
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- unaffected
firefox80 --- unaffected
firefox81 --- unaffected
firefox82 --- fixed

People

(Reporter: denschub, Assigned: agi)

References

(Regression)

Details

(Keywords: regression)

Attachments

(3 files, 1 obsolete file)

Bug 1664522 - Fix tabs.captureVisibleTab for mobile.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1664522 - Implement tabs.captureTab on mobile.
47 bytes, text/x-phabricator-request
Details | Review
Bug 1664522 - Move ext_tabs_captureTab test to toolkit.
47 bytes, text/x-phabricator-request
Details | Review

On a Gecko 82 build, calling tabs.captureVisibleTab() fails with the following exception:

[JavaScript Error: "this.nativeTab.ownerGlobal is undefined" {file: "chrome://extensions/content/parent/ext-tabs-base.js" line: 126}]
capture@chrome://extensions/content/parent/ext-tabs-base.js:126:45
captureVisibleTab@chrome://geckoview/content/ext-tabs.js:480:22
Async*recvAPICall/result</<@resource://gre/modules/ExtensionParent.jsm:951:68
withPendingBrowser@resource://gre/modules/ExtensionParent.jsm:489:26
recvAPICall/result<@resource://gre/modules/ExtensionParent.jsm:951:24
callAndLog@resource://gre/modules/ExtensionParent.jsm:913:14
recvAPICall@resource://gre/modules/ExtensionParent.jsm:950:25
Async*_recv@resource://gre/modules/ConduitsChild.jsm:78:20
receiveMessage@resource://gre/modules/ConduitsParent.jsm:371:20
JSActor query*_send@resource://gre/modules/ConduitsChild.jsm:63:11
_send@resource://gre/modules/ConduitsChild.jsm:111:18
callParentAsyncFunction@resource://gre/modules/ExtensionChild.jsm:857:18
callAsyncFunction@resource://gre/modules/ExtensionChild.jsm:621:33
stub@resource://gre/modules/Schema

This works on a beta (81) build, and :agi mentioned that there have been some code changes to that API recently, so this is a new regression.

Oops, sorry about that. The nativeGlobal there is the chrome window on Desktop, not sure what it would be on android.

This was regressed by the first patch from bug 1636508 that landed 17 days ago -- the line from the call stack is now a comment after the second patch landed yesterday.

Another thing that might be different on Fenix is the ZoomManager, not sure if that's fully wired up on android. Also, I was updating mobile tests, expecting they were running and actually testing things :/ could you please also verify what's going on with that?

Let me know if I can be of any help!

No worries!

this.browser.ownerGlobal is the equivalent for mobile.

ZoomManager is not available yeah, I think we can find a replacement for it, I'll put up a patch soon.

The test passes somehow 🤷‍♂️ looking into what's going on there

Ok so it turns out getting screenshots of desktop viewport pages is broken, but mobile viewport works as expected. Since the mobile viewport case is more important I just opened Bug 1664522

There are two things wrong with captureVisibleTab on mobile:

  • ownerGlobal is not available on nativeTab, this patch adds a new getter that
    returns the browser's ownerGlobal.
  • ZoomManager is not available on mobile, this patch adds a fallback.

The test was also not actually running due to a refuse in
https://hg.mozilla.org/mozilla-central/rev/56c287941f2a60a7ea18baf45ffc526c4cd137e2,
this patch fixes that by moving the runTest function to be async instead of a
generator.

While working on this patch I noticed that the captureVisibleTab API only works
for mobile viewports, so I added a test and opened a bug for that.

Assignee: nobody → agi
Status: NEW → ASSIGNED

(In reply to Agi Sferro | :agi | ⏰ PST | he/him from comment #6)

Ok so it turns out getting screenshots of desktop viewport pages is broken, but mobile viewport works as expected. Since the mobile viewport case is more important I just opened Bug 1664522

bug 1664594 *

See Also: → 1664594

There are two things wrong with captureVisibleTab on mobile:

* ownerGlobal is not available on nativeTab, this patch uses
  this.browser.ownerGlobal instead.
* ZoomManager is not available on mobile, this patch moves that code to
  browser/../ext-tabs.
Attachment #9175620 - Attachment is obsolete: true
Pushed by asferro@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c951834c99c7
Implement tabs.captureTab on mobile. r=snorp
https://hg.mozilla.org/integration/autoland/rev/04b61d3d734a
Fix tabs.captureVisibleTab for mobile. r=robwu,zombie,snorp
https://hg.mozilla.org/integration/autoland/rev/30b8a0e3514b
Move ext_tabs_captureTab test to toolkit. r=robwu,snorp
Backout by dluca@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/515cce30a345
Backed out 3 changesets for Mochitest failures in extensions/test/mochitest/test_ext_tabs_captureTab.html. CLOSED TREE
Severity: -- → S2
Priority: -- → P2
Pushed by asferro@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/af27b96245a0
Implement tabs.captureTab on mobile. r=snorp
https://hg.mozilla.org/integration/autoland/rev/810036a8a9f7
Fix tabs.captureVisibleTab for mobile. r=robwu,zombie,snorp
https://hg.mozilla.org/integration/autoland/rev/ad4be03cc660
Move ext_tabs_captureTab test to toolkit. r=robwu,snorp
You need to log in before you can comment on or make changes to this bug.