Closed Bug 882132 Opened 6 years ago Closed 6 years ago

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

Categories

(Firefox Graveyard :: SocialAPI, defect)

defect
Not set

Tracking

(firefox23 fixed, firefox24 fixed)

RESOLVED FIXED
Firefox 24
Tracking Status
firefox23 --- fixed
firefox24 --- fixed

People

(Reporter: standard8, Assigned: scaraveo)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file)

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?
(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
Flags: needinfo?(mbanner)
(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.
(In reply to :Gavin Sharp (use gavin@gavinsharp.com 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).
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)
Attachment #762964 - Flags: review?(felipc)
Flags: needinfo?(mbanner)
Shouldn't you also remove the XXX comment now?
Just to note, I tested this yesterday and it worked fine for us.
(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?
Flags: needinfo?(mbanner)
Sorry for not being clear. It worked with the patch only. Without the patch it doesn't work.
Flags: needinfo?(mbanner)
Attachment #762964 - Flags: review?(felipc) → review+
https://hg.mozilla.org/mozilla-central/rev/aded0559ac48
Assignee: nobody → scaraveo
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 24
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
Attachment #762964 - Flags: approval-mozilla-aurora?
Attachment #762964 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Deprioritizing verification since this landed with tests. Please drop the [qa-] tag and add the verifyme keyword of QA verification is needed.
Whiteboard: [qa-]
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.