Closed Bug 1688476 Opened 3 years ago Closed 3 years ago

Port bug 1679688 to Thunderbird - make host permissions grant access to privileged parts of the tabs API

Categories

(Thunderbird :: Upstream Synchronization, task, P1)

Tracking

(thunderbird_esr78 unaffected, thunderbird86 fixed)

RESOLVED FIXED
87 Branch
Tracking Status
thunderbird_esr78 --- unaffected
thunderbird86 --- fixed

People

(Reporter: mkmelin, Assigned: TbSync)

References

Details

Attachments

(1 file, 2 obsolete files)

Pretty sure this is what's causing all the orange WX tests.

https://hg.mozilla.org/mozilla-central/rev/a5b3fa068d5e

Priority: -- → P1

Multiple things happen here.

  1. This change in ext-tabs-base.js is causing our mailTabs.query() function to return empty, causing almost all of our tests to fail.
    https://hg.mozilla.org/mozilla-central/diff/a5b3fa068d5ee122dac27298d7da7d7231d3db49/toolkit/components/extensions/parent/ext-tabs-base.js#l1.67

We need to add title: null to here:
https://searchfox.org/comm-central/source/mail/components/extensions/parent/ext-mailTabs.js#169

  1. The new get matchesHostPermission() function fails when checking our editor#content-frame tabs and our address book.
    https://searchfox.org/mozilla-central/source/toolkit/components/extensions/parent/ext-tabs-base.js#203

When debugging and checking the value for this.browser.currentURI.spec I get about:blank?compose, checking this._url returns undefined, which I do not understand, as it is defined a few lines below as:
https://searchfox.org/mozilla-central/source/toolkit/components/extensions/parent/ext-tabs-base.js#221-223

Replacing this._url by this.browser.currentURI.spec fixes these errors for the editor tabs, but not for the address book, as this.browser is not defined. To fix this I just return false in my debug patch, if the function fails for any reason.

This fixes the failing mailTabs.query

Assignee: nobody → john

This is my debug patch for M-C to get our tests working again.

To fix that in C-C we need to make sure all our tabs have a browser with a currentURI defined. This is currently not the case for our address book (and maybe more).

Furthermore, we must ensure, that calls to this._url properly returns the URL. This currently fails mysteriously for editor#content-frame, even though this.browser.currentURI.spec (which is this._url) returns something useful (about:blank?compose).

Should we file a patch at m-c which failsafes matchesHostPermission() and maybe all calls which use this.browser, or should we fix that in c-c?

Flags: needinfo?(geoff)

To fix that in C-C we need to make sure all our tabs have a browser with a currentURI defined. This is currently not the case for our address book (and maybe more).

This problem has been mentioned numerous times without resolution. For example, see Bug 1649035 and Bug 1680587.

Thanks for the tip, Geoff!

Attachment #9199040 - Attachment is obsolete: true
Attachment #9199045 - Attachment is obsolete: true
Flags: needinfo?(geoff)
Attachment #9199112 - Flags: review?(geoff)

Comment on attachment 9199112 [details] [diff] [review]
bug1688476_fix_mailTabs.query_and_TabmailTab.TabmailTab.matchesHostPermission.patch

The comment should have a . and one fewer * at the end, but I'll do it when I land this shortly.

Attachment #9199112 - Flags: review?(geoff) → review+

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/0115f45d6db7
Fix mailTabs.query() and Tab.matchesHostPermission(). r=darktrojan

Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 87 Branch

Comment on attachment 9199112 [details] [diff] [review]
bug1688476_fix_mailTabs.query_and_TabmailTab.TabmailTab.matchesHostPermission.patch

[Approval Request Comment]
Per Geoff, this will fix errors seen on early builds of what will be 86.0b1.

Attachment #9199112 - Flags: approval-comm-beta?

Comment on attachment 9199112 [details] [diff] [review]
bug1688476_fix_mailTabs.query_and_TabmailTab.TabmailTab.matchesHostPermission.patch

[Triage Comment]
definitely approved for beta :)

Attachment #9199112 - Flags: approval-comm-beta? → approval-comm-beta+
You need to log in before you can comment on or make changes to this bug.