Closed Bug 1544793 Opened 1 year ago Closed 7 months ago

win.window.gBrowser is undefined

Categories

(Thunderbird :: Add-Ons: Extensions API, defect)

defect
Not set

Tracking

(thunderbird_esr6869+ fixed, thunderbird70 fixed)

RESOLVED FIXED
Thunderbird 70.0
Tracking Status
thunderbird_esr68 69+ fixed
thunderbird70 --- fixed

People

(Reporter: fsteiner-board, Assigned: darktrojan)

Details

Attachments

(2 files)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:66.0) Gecko/20100101 Firefox/66.0

Steps to reproduce:

I tried to add a title prefix with the following code (as nightly tester tools could do in the past).

const update_titlebar = async () => {
const windows = await browser.windows.getAll();
windows.forEach(async win => {
await browser.windows.update(win.id, { width: 1000, titlePreface: "bla" });
});
};

and let it fire e.g. by
browser.tabs.onCreated.addListener(async () => await update_titlebar());

The witdh part is only there to see if the browser.windows.update works at all.

Actual results:

The width resize does work, but using the titlePreface shows the following error in the debugger console:

win.window.gBrowser is undefined ext-windows.js:196
update chrome://messenger/content/parent/ext-windows.js:196
update self-hosted:1005
result resource://gre/modules/ExtensionParent.jsm:950
withPendingBrowser resource://gre/modules/ExtensionParent.jsm:604
result resource://gre/modules/ExtensionParent.jsm:949
withTiming resource://gre/modules/ExtensionParent.jsm:916
call resource://gre/modules/ExtensionParent.jsm:948
AsyncFunctionNext self-hosted:839
Error: An unexpected error occurred undefined

Expected results:

Should have set the title prefix. I got it from https://thunderbird-webextensions.readthedocs.io/en/latest/windows.html and thought that things described there are already implemented. But likely this is kind of work-in-progress?

Still there in 67b3.

Code doesn't match documentation? Related to bug 1575693 which just got fixed (events didn't fire)?

Flags: needinfo?(john.bieling)
Flags: needinfo?(geoff)

That is part of the WebExtension documentation, which is done by Geoff. I am of no help here.

Flags: needinfo?(john.bieling)

(In reply to Frank Steiner from comment #0)

Should have set the title prefix. I got it from https://thunderbird-webextensions.readthedocs.io/en/latest/windows.html and thought that things described there are already implemented. But likely this is kind of work-in-progress?

No, it's just a bug that needs to be fixed. It seems like it should be simple, so I'll look at it shortly.

Depends on bug 1575710 since otherwise I'd have to change it too.

Assignee: nobody → geoff
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(geoff)
Attachment #9088045 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9088045 [details] [diff] [review]
1544793-window-title-1.diff

Review of attachment 9088045 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM. r=mkmelin
Attachment #9088045 - Flags: review?(mkmelin+mozilla) → review+
Keywords: checkin-needed

Oh, depends on bug 1575710.

Keywords: checkin-needed

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/71cd49851c2d
Fix setting window title preface by windows API; r=mkmelin DONTBUILD

Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 70.0
Comment on attachment 9088045 [details] [diff] [review]
1544793-window-title-1.diff

Wanted in 68.2.
Attachment #9088045 - Flags: approval-comm-esr68?

We're still doing fixes for TB 68.1, see https://mzl.la/2jV90KX and bug 1577706. So is this so risky or unimportant that it wouln't go there?

Flags: needinfo?(geoff)

I'm in no rush for it and it doesn't apply without the bugs I landed it with.

Flags: needinfo?(geoff)

What is that meant to mean? Which bugs? Bug 1575708 and bug 1575710? Those won't be backported. I don't see any dependents here? So this needs a special TB 68 ESR patch? I only see a conflict in mail/base/content/tabmail.js and that's most likely due to prettier.

Flags: needinfo?(geoff)

Sorry, I saw the uplift request for the other two now.

Flags: needinfo?(geoff)
Attachment #9088045 - Flags: approval-comm-esr68? → approval-comm-esr68+
Comment on attachment 9088045 [details] [diff] [review]
1544793-window-title-1.diff

Needs rebasing just like bug 1575710 and bug 1575708.
Flags: needinfo?(geoff)
Attachment #9088045 - Flags: approval-comm-esr68+
Comment on attachment 9088045 [details] [diff] [review]
1544793-window-title-1.diff

Sorry, this is a prettified patch which applies after prettifying c-esr68.
Flags: needinfo?(geoff)
Attachment #9088045 - Flags: approval-comm-esr68+
Comment on attachment 9088045 [details] [diff] [review]
1544793-window-title-1.diff

Sorry, this needs rebasing. The patch is for mail/base/content/tabmail.js but in TB 68 that's still mail/base/content/tabmail.xml.

I don't know how to translate `setDocumentTitle(aTab = this.selectedTab) {` to the XML equivalent:
```
      <method name="setDocumentTitle">
        <parameter name="aTab"/>
```
Maybe open code it? `if (!aTab) aTab = this.selectedTab`. I'll leave that to you. Just this hunk doesn't apply.
Flags: needinfo?(geoff)
Attachment #9088045 - Flags: approval-comm-esr68+
Comment on attachment 9088045 [details] [diff] [review]
1544793-window-title-1.diff

Setting the flag so we won't forget.
Attachment #9088045 - Flags: approval-comm-esr68?

Here's the rebased patch. I discovered there's a problem with the original – an empty string should clear the preface but doesn't – so I've fixed it here and will post another for central.

Flags: needinfo?(geoff)
Attachment #9094480 - Flags: approval-comm-esr68?
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/145ab0ce87cf
follow-up - Explicitly check for null instead of falsy value; rs=me DONTBUILD
Attachment #9088045 - Flags: approval-comm-esr68?
Comment on attachment 9094480 [details] [diff] [review]
1544793-window-title-esr1.diff

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