Closed Bug 1674407 Opened 11 months ago Closed 11 months ago

tabs.create() should wait for window load

Categories

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

defect

Tracking

(thunderbird_esr78 fixed, thunderbird83 fixed)

RESOLVED FIXED
84 Branch
Tracking Status
thunderbird_esr78 --- fixed
thunderbird83 --- fixed

People

(Reporter: TbSync, Assigned: TbSync)

References

Details

Attachments

(2 files)

A call to browser.tabs.create during Thunderbird start fails with an error about tabmail not being defined.

https://thunderbird.topicbox.com/groups/addons/T5e49f3ac49bed9a1-Mcfba2a4d42d7d2b36ce290d6/how-to-open-a-content-tab-at-startup

This works as expected in Firefox. The difference is, that the firefox implementation is waiting for browser-delayed-startup-finished.

We have something similar "mail-delayed-startup-finished" and we should use that.

Attached patch bug1674407.patchSplinter Review

Patch using the same method as firefox, but with the "mail-delayed-startup-finished" notification.

Attachment #9184808 - Flags: review?(mkmelin+mozilla)
Status: NEW → ASSIGNED

(In reply to John Bieling (:TbSync) from comment #0)

A call to browser.taps.create during Thunderbird start fails with an error about tabmail not being defined.

Well, a call to browser.taps.create will always fail becaused "tabs" is misspelled... ;-)

Summary: taps.create should wait for window load → tabs.create() should wait for window load
Comment on attachment 9184808 [details] [diff] [review]
bug1674407.patch

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

Looks good, thx! r=mkmelin
Attachment #9184808 - Flags: review?(mkmelin+mozilla) → review+

Do you have a try run?
Please set affected flags for 78 and beta.

Target Milestone: --- → 84 Branch

Doesn't look like any new oranges, which is what to look out for.

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/38bd4260fb50
tabs.create() should wait for window load. r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED

Comment on attachment 9184808 [details] [diff] [review]
bug1674407.patch

[Approval Request Comment]
User impact if declined: Cannot use browser.tabs.create() during Thunderbird start
Testing completed (on c-c, etc.): OK
Risk to taking this patch (and alternatives if risky): None

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

Comment on attachment 9184808 [details] [diff] [review]
bug1674407.patch

[Triage Comment]
Approved for beta

Attachment #9184808 - Flags: approval-comm-beta? → approval-comm-beta+

Comment on attachment 9184808 [details] [diff] [review]
bug1674407.patch

[Approval Request Comment]
User impact if declined: Cannot use browser.tabs.create() during Thunderbird start
Testing completed (on c-c, etc.): OK
Risk to taking this patch (and alternatives if risky): Noneatch (and alternatives if risky):

Attachment #9184808 - Flags: approval-comm-esr78?

Comment on attachment 9184808 [details] [diff] [review]
bug1674407.patch

[Triage Comment]
Approved for esr78

Attachment #9184808 - Flags: approval-comm-esr78? → approval-comm-esr78+

Causes test failures on esr78:
https://treeherder.mozilla.org/jobs?repo=comm-esr78&selectedTaskRun=VkCn9OXwSmiCTLVGxP74vw.0

Backing this bug out "fixed" the tests, but they should probably be made to work rather than backing out.

Flags: needinfo?(mkmelin+mozilla)
Flags: needinfo?(john)

I will check the test.

Flags: needinfo?(john)
Flags: needinfo?(mkmelin+mozilla)

I was able to build ESR and the result is not nice: The patch works on trunk but not on ESR. It is not just the test, it really does not work. I have to investigate.

It looks like this patch is still on comm-esr78 ? If so, it has to be removed before release of 78.5

Flags: needinfo?(rob)

Fail in ESR78 is due to bug 1660904 not being uplifted.

Flags: needinfo?(rob)

This fixes the problems for this bug on 78. We're waiting for a notification that never arrives, because it doesn't exist on 78. Not really any risk, but I've asked Magnus for review anyway.

Attachment #9187958 - Flags: review?(mkmelin+mozilla)
Attachment #9187958 - Flags: approval-comm-esr78?
Comment on attachment 9187958 [details] [diff] [review]
1674407-esr78-fixes.diff

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

LGMT, r=mkmelin
Attachment #9187958 - Flags: review?(mkmelin+mozilla) → review+

Comment on attachment 9187958 [details] [diff] [review]
1674407-esr78-fixes.diff

[Triage Comment]
ESR78 fixes for previously approved changes.

Attachment #9187958 - Flags: approval-comm-esr78? → approval-comm-esr78+
Duplicate of this bug: 1653497
You need to log in before you can comment on or make changes to this bug.