Closed Bug 940494 Opened 7 years ago Closed 7 years ago

when a Social provider has a status button, the provider's icon should go to the status button rather than the social api toolbar button

Categories

(Firefox Graveyard :: SocialAPI, defect)

26 Branch
defect
Not set
normal

Tracking

(firefox26 wontfix, firefox27+ fixed, firefox28 fixed)

RESOLVED FIXED
Firefox 28
Tracking Status
firefox26 --- wontfix
firefox27 + fixed
firefox28 --- fixed

People

(Reporter: mixedpuppy, Assigned: florian)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file, 1 obsolete file)

In certain situations, a provider without a sidebar shows up in sidebar selection, and notifications go to that rather than the status button.
Attached patch WIP (obsolete) — Splinter Review
This patch gives a mostly reasonable behavior for handling of the icons (the icons of social providers providing a statusURL go only to their status button).
Comment on attachment 8336116 [details] [diff] [review]
WIP

This looks good and addresses a couple edge cases I had noticed before.  While were removing the toolbarbutton for 28 (assuming I can land that) this is a good fix for 27 if we can uplift.
Attachment #8336116 - Flags: feedback+
Attached patch PatchSplinter Review
Same patch as before, but now the tests pass.
Assignee: nobody → florian
Attachment #8336116 - Attachment is obsolete: true
Attachment #8336880 - Flags: review?(mhammond)
Rephrasing bug summary to reflect the issue we are actually addressing here. We may deal with the provider selection menu later; but it seems less important. The patch here contains the changes we really need for Firefox 27 (aurora). This patch obviously bitrots with the patches in bug 935640, so depending on what happens there and when, we may or may not land this on mozilla-central too.
Summary: sometimes a provider shows up in sidebar selection when not providing a sidebar → when a Social provider has a status button, the provider's icon should go to the status button rather than the social api toolbar button
Blocks: Talkilla
Attachment #8336880 - Flags: review?(mhammond) → review+
(In reply to Florian Quèze [:florian] [:flo] from comment #4)

> The patch here contains the changes we really need for Firefox 27
> (aurora). This patch obviously bitrots with the patches in bug 935640, so
> depending on what happens there and when, we may or may not land this on
> mozilla-central too.

I discussed this with Shane Friday evening, given that:
- some of the changes here are needed for Firefox 28,
- having the patch on mozilla-central could simplify the uplift,
- Shane's patches in bug 935640 haven't been reviewed yet and Shane doesn't mind dealing with a touch of bitrot...
I decided to land this for Firefox 28:
https://hg.mozilla.org/integration/fx-team/rev/ffd73f058358
OS: Mac OS X → All
Hardware: x86 → All
https://hg.mozilla.org/mozilla-central/rev/ffd73f058358
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 28
Comment on attachment 8336880 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): The feature (SocialAPI status buttons) was implemented in bug 891225 and enabled in bug 906839.
User impact if declined: Very confusing icons on social toolbar buttons for social providers using the new status panel feature.
Testing completed (on m-c, etc.): We confirmed that today's m-c nightly with the patch fixes the issue.
Risk to taking this patch (and alternatives if risky): low.
String or IDL/UUID changes made by this patch: none.
Attachment #8336880 - Flags: approval-mozilla-aurora?
Attachment #8336880 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
I don't think this needs QA verification. If anyone thinks that's a mistake please remove the [qa-] whiteboard tag and add the verifyme keyword.
Whiteboard: [qa-]
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.