Last Comment Bug 804736 - Allow reactivation from webpages of Social API
: Allow reactivation from webpages of Social API
Status: VERIFIED FIXED
[Fx17]
:
Product: Firefox
Classification: Client Software
Component: SocialAPI (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 19
Assigned To: Jared Wein [:jaws] (please needinfo? me)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-23 13:23 PDT by Jared Wein [:jaws] (please needinfo? me)
Modified: 2012-12-04 15:18 PST (History)
6 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
verified
+
verified
verified


Attachments
Patch (1.04 KB, patch)
2012-10-23 13:23 PDT, Jared Wein [:jaws] (please needinfo? me)
mixedpuppy: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
lukasblakk+bugs: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Jared Wein [:jaws] (please needinfo? me) 2012-10-23 13:23:42 PDT
Created attachment 674357 [details] [diff] [review]
Patch

Bug 764869 implemented activation from content but required that the provider had not been previously activated before.

The attached patch will allow activation as long as the Social API is not already enabled, which seems the more correct scenario.

social.enabled implies social.active.
social.active does not imply social.enabled.
Comment 1 :Felipe Gomes (needinfo me!) 2012-10-23 14:44:08 PDT
Comment on attachment 674357 [details] [diff] [review]
Patch

redirecting to shane
Comment 2 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-10-23 15:08:02 PDT
The original intent here was that we didn't want to allow the provider to re-enable if the user had previously enabled it, but then actively disabled it (i.e. via the Tools menu), because there was some potential for abuse there (e.g. if the provider just fired the event on every page view, that would get annoying really quickly).

It's worth re-visiting since we've gotten feedback from Facebook that they'd like to have a way to offer to re-enabled the feature, which seems reasonable, but we shouldn't lose track of the possible downsides here. I'd like to get feedback from others before moving ahead with this.
Comment 3 Shane Caraveo (:mixedpuppy) 2012-10-23 15:12:22 PDT
I seem to keep running into issues like this...we need to know if the user took some action in content (e.g. event.isTrusted) without having to watch for multiple events.  Popup blocking does this, but not in a generic fashion.

I think in the case of a whitelisted provider, we have a relationship where we should be able to trust the provider to do the right thing.
Comment 4 Shane Caraveo (:mixedpuppy) 2012-10-23 15:33:06 PDT
Comment on attachment 674357 [details] [diff] [review]
Patch

This would be fine, pending some thought around avoiding abuse.  As well, we'll ensure that active is always true if enabled in bug 804242
Comment 5 Jared Wein [:jaws] (please needinfo? me) 2012-10-25 12:35:59 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd7180e29211
Comment 6 Jared Wein [:jaws] (please needinfo? me) 2012-10-25 12:36:43 PDT
Comment on attachment 674357 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a
User impact if declined: if a user deactivates the social api, they cannot reactivate from the social providers site anymore
Testing completed (on m-c, etc.): locally, just landed on inbound
Risk to taking this patch (and alternatives if risky): none expected
String or UUID changes made by this patch: none
Comment 7 Ryan VanderMeulen [:RyanVM] 2012-10-25 18:27:16 PDT
https://hg.mozilla.org/mozilla-central/rev/fd7180e29211
Comment 9 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-12-04 15:18:38 PST
Verified fixed across all branches.

Note You need to log in before you can comment on or make changes to this bug.