882132 - Posting social.user-profile with changed iconURL doesn't update the toolbar icon until reload of the provider

Status: RESOLVED FIXED

(Firefox Graveyard :: SocialAPI, defect)

Description

[Mark Banner (:standard8)]

I'm trying to get the toolbar icon to change when we send a user-profile message. The intended use is that we'll badge the icon with some indication of offline/online state.

However, at the moment, even though I've sent a user-profile message, it only changes the icon when I reload the sidebar, not when I send the message.

Looking into the code I found:

http://hg.mozilla.org/mozilla-central/annotate/9ca690835a5e/browser/base/content/browser-social.js#l1076

// XXX doesn't this need to be called for profile changes, given its use of provider.profile?

Comment 1

[Shane Caraveo (:mixedpuppy)]

(In reply to Mark Banner (:standard8) from comment #0)
> I'm trying to get the toolbar icon to change when we send a user-profile
> message. The intended use is that we'll badge the icon with some indication
> of offline/online state.
> 
> However, at the moment, even though I've sent a user-profile message, it
> only changes the icon when I reload the sidebar, not when I send the message.
> 
> Looking into the code I found:
> 
> http://hg.mozilla.org/mozilla-central/annotate/9ca690835a5e/browser/base/
> content/browser-social.js#l1076
> 
> // XXX doesn't this need to be called for profile changes, given its use of
> provider.profile?

I'm unable to reproduce this.  That comment will need to be removed.  The profile-changed message does indeed update the toolbar button.

http://hg.mozilla.org/mozilla-central/annotate/9ca690835a5e/browser/base/content/browser-social.js#l115

Comment 2

[:Gavin Sharp [email: [e-mail]]]

(In reply to Shane Caraveo (:mixedpuppy) from comment #1)
> The
> profile-changed message does indeed update the toolbar button.
> 
> http://hg.mozilla.org/mozilla-central/annotate/9ca690835a5e/browser/base/
> content/browser-social.js#l115

It calls updateProfile(), but that doesn't call updateButton() AFAICT.

Comment 3

[Shane Caraveo (:mixedpuppy)]

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #2)
> (In reply to Shane Caraveo (:mixedpuppy) from comment #1)
> > The
> > profile-changed message does indeed update the toolbar button.
> > 
> > http://hg.mozilla.org/mozilla-central/annotate/9ca690835a5e/browser/base/
> > content/browser-social.js#l115
> 
> It calls updateProfile(), but that doesn't call updateButton() AFAICT.

ah, I misinterpreted what they were trying to do.  I thought they were trying to update the users image, but they want to change the provider icon.  I see we have code for that in the provider class, but it seems odd to update that via user-profile (thus my confusion).

Comment 4

[Shane Caraveo (:mixedpuppy)]

Attachment: fix updating provider icon

fixes updating the provider icon.  updateProvider also ultimately calls updateProfile.  also removes external url from test (even though it wasn't used in any test)

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Shouldn't you also remove the XXX comment now?

Comment 6

[Mark Banner (:standard8)]

Just to note, I tested this yesterday and it worked fine for us.

Comment 7

[Shane Caraveo (:mixedpuppy)]

(In reply to Mark Banner (:standard8) from comment #6)
> Just to note, I tested this yesterday and it worked fine for us.

Mark, do you mean it worked with or without this patch?

Comment 8

[Mark Banner (:standard8)]

Sorry for not being clear. It worked with the patch only. Without the patch it doesn't work.

Comment 9

[Shane Caraveo (:mixedpuppy)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/aded0559ac48

Comment 10

[Ryan VanderMeulen [:RyanVM]]

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

Comment 11

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 762964 [details] [diff] [review]
fix updating provider icon

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 809694
User impact if declined: api was not fully implemented, resulting in providers being unable to update one of their icons.  we need this for some webrtc apps work.
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a036f999803f

Comment 13

[Anthony Hughes (:ashughes) [QA]]

Deprioritizing verification since this landed with tests. Please drop the [qa-] tag and add the verifyme keyword of QA verification is needed.