The default bug view has changed. See this FAQ.

AppMenu button doesn't get added to Chat tab automatically

RESOLVED FIXED in Thunderbird 19.0

Status

Thunderbird
Instant Messaging
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mconley, Assigned: mconley)

Tracking

17 Branch
Thunderbird 19.0
x86
All

Thunderbird Tracking Flags

(thunderbird17+ fixed, thunderbird18 fixed)

Details

Attachments

(2 attachments, 3 obsolete attachments)

I just noticed that even though bug 785692 landed, we didn't migrate the AppMenu button onto the chat tab.

We should do that.

I'll try to come up with a patch today.
Created attachment 672304 [details] [diff] [review]
Patch v1

First go at this.
Comment on attachment 672304 [details] [diff] [review]
Patch v1

Look OK?
Attachment #672304 - Flags: review?(florian)
Comment on attachment 672304 [details] [diff] [review]
Patch v1

Looks OK to me, but I think you will want a reviewer who knows this code a bit better to double check.
Attachment #672304 - Flags: review?(florian)
Attachment #672304 - Flags: review?
Attachment #672304 - Flags: feedback+
Comment on attachment 672304 [details] [diff] [review]
Patch v1

The only one I can think of would be Blake...or maybe Magnus, or Sid. I'll try Blake first.
Attachment #672304 - Flags: review? → review?(bwinton)
What about the people who worked on bug 785692? Paenglab maybe?
(In reply to Florian Quèze [:florian] [:flo] from comment #5)
> What about the people who worked on bug 785692? Paenglab maybe?

I'll Cc him to see what he thinks.
For me it looks good and this is what we missed on the migration.
Comment on attachment 672304 [details] [diff] [review]
Patch v1

Blake is tied up. Magnus, do you have a few minutes to review me?
Attachment #672304 - Flags: review?(bwinton) → review?(mkmelin+mozilla)

Comment 9

5 years ago
Comment on attachment 672304 [details] [diff] [review]
Patch v1

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

Looks good to me. r=mkmelin

::: mail/base/modules/mailMigrator.js
@@ +244,5 @@
> +         */
> +        let addButtonToEnd = function(aToolbarID, aButtonID) {
> +          let barResource = this._rdf.GetResource(MESSENGER_DOCURL +
> +                                                  aToolbarID);
> +          if (barResource !== null) {

shouldn't this just be if (barResource) ?
Attachment #672304 - Flags: review?(mkmelin+mozilla) → review+
Created attachment 672451 [details] [diff] [review]
Patch v2 (r+'d by mkmelin, feedback+ from florian)

This got forgotten when we put in the AppMenu.

With this patch, users on Beta won't get migrated forward...we *could* add a new migration in that case. Let me know what's preferred.
Attachment #672304 - Attachment is obsolete: true
Attachment #672451 - Flags: approval-comm-beta?
Attachment #672451 - Flags: approval-comm-aurora?
Comment on attachment 672451 [details] [diff] [review]
Patch v2 (r+'d by mkmelin, feedback+ from florian)

Don't forget to land this on trunk as well.
Attachment #672451 - Flags: approval-comm-beta?
Attachment #672451 - Flags: approval-comm-beta+
Attachment #672451 - Flags: approval-comm-aurora?
Attachment #672451 - Flags: approval-comm-aurora+
Created attachment 673855 [details] [diff] [review]
Checked in patch

Thanks - I made the change that Magnus suggested. This is the version of the patch that got checked in.
Attachment #672451 - Attachment is obsolete: true
comm-central: https://hg.mozilla.org/comm-central/rev/ecc2d449db40
comm-aurora: https://hg.mozilla.org/releases/comm-aurora/rev/6e059f816a3e
comm-beta: https://hg.mozilla.org/releases/comm-beta/rev/d450fefad0d0
Status: NEW → RESOLVED
Last Resolved: 5 years ago
status-thunderbird17: --- → fixed
status-thunderbird18: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 19.0
Backed out due to oranges on Linux:

comm-central: https://hg.mozilla.org/comm-central/rev/2c84c87d353c
comm-aurora: https://hg.mozilla.org/releases/comm-aurora/rev/e9502f3d7590
comm-beta: https://hg.mozilla.org/releases/comm-beta/rev/4178cdbe1daf
Status: RESOLVED → REOPENED
status-thunderbird17: fixed → affected
status-thunderbird18: fixed → affected
Resolution: FIXED → ---
tracking-thunderbird17: --- → +
Created attachment 674680 [details] [diff] [review]
Possible test fix

It's not clear to me what was happening on our Mozmill machines when I first landed the patch.

My suspicion is that my menubar tests were running so quickly that Thunderbird started to shut down before it had finished starting up. So I've added some waits to ensure that TB has finished starting.

Whether or not this actually helps is unclear, but it certainly doesn't hurt.
Attachment #674680 - Flags: review?(mbanner)
Created attachment 674704 [details] [diff] [review]
Possible test fix (for real)

The real patch this time.
Attachment #674680 - Attachment is obsolete: true
Attachment #674680 - Flags: review?(mbanner)
Attachment #674704 - Flags: review?(mbanner)
Comment on attachment 674704 [details] [diff] [review]
Possible test fix (for real)

Yep, lets try it, although if it works, we could also argue that we should be waiting for mail-startup-done anyway...
Attachment #674704 - Flags: review?(mbanner) → review+
Attachment #674704 - Flags: approval-comm-beta?
Attachment #674704 - Flags: approval-comm-aurora?
Attachment 673855 [details] [diff]
comm-central: https://hg.mozilla.org/comm-central/rev/5ba465f0c7f0
comm-aurora: https://hg.mozilla.org/releases/comm-aurora/rev/5172c9cf6417
comm-beta: https://hg.mozilla.org/releases/comm-beta/rev/40b0e18f5f7e

Attachment 674704 [details] [diff]
comm-central: https://hg.mozilla.org/comm-central/rev/ce1bbebaef20
comm-aurora: https://hg.mozilla.org/releases/comm-aurora/rev/994992a767aa
comm-beta: https://hg.mozilla.org/releases/comm-beta/rev/97dd0c4aadee
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
status-thunderbird17: affected → fixed
status-thunderbird18: affected → fixed
Resolution: --- → FIXED
Attachment #674704 - Flags: approval-comm-beta?
Attachment #674704 - Flags: approval-comm-beta+
Attachment #674704 - Flags: approval-comm-aurora?
Attachment #674704 - Flags: approval-comm-aurora+
Attachment #673855 - Flags: approval-comm-beta+
Attachment #673855 - Flags: approval-comm-aurora+
You need to log in before you can comment on or make changes to this bug.