Closed Bug 1240682 Opened 4 years ago Closed 4 years ago

social addons often have int versions rather than string versions

Categories

(Firefox Graveyard :: SocialAPI: Providers, defect)

defect
Not set

Tracking

(firefox45 fixed, firefox46 fixed)

RESOLVED FIXED
Firefox 46
Tracking Status
firefox45 --- fixed
firefox46 --- fixed

People

(Reporter: Dexter, Assigned: mixedpuppy)

Details

Attachments

(1 file)

From bug 1238142 comment 5:

"[...] it looks like the version returned from the addon manager should really be a string [0]. SocialService.jsm seems to assume [1] it is (see the "" part), but the Facebook social manifest has a number in it:

{
        ... other removed stuff ...
	"shareURL" : "https://www.facebook.com/sharer/sharer.php?u=%{url}",
	"version" : 2,
	"postActivationURL" : "https://activations.cdn.mozilla.net/en-US/activated/facebook.html",
	"updateDate" : 1452587419933,
	"installDate" : 1452534699655
}

The examples in the SocialAPI manifest documentation [2] seem to imply that version should be a string as well.

[0] - https://developer.mozilla.org/en-US/Add-ons/Add-on_Manager/Addon
[1] - https://dxr.mozilla.org/mozilla-central/rev/5acc2a44834ce0614f98466475e674517daf0041/toolkit/components/social/SocialService.jsm#1156
[2] - https://developer.mozilla.org/en-US/docs/Mozilla/Projects/Social_API/Manifest"
Shane, do we have any control on this manifest? Would it be possible to fix the version string to be a string? This would allow us to have less broken data in Telemetry until the fix gets to the Release channel.
Flags: needinfo?(mixedpuppy)
Whiteboard: [measurement:client:tracking]
Flags: needinfo?(mixedpuppy)
The only way to fix this is ensure data coming into the addon layer is correct.  This patch would fix the issue.
Summary: The version for the Facebook provider manifest should be a string → social addons often have int versions rather than string versions
(In reply to Shane Caraveo (:mixedpuppy) from comment #3)
> The only way to fix this is ensure data coming into the addon layer is
> correct.  This patch would fix the issue.

I see, this makes sense. Is there any possibility to at least fix the Facebook manifest (to reduce the data outage in Telemetry while the fix rides the trains)? Is it our of our control (i.e. handled by Facebook)?
(In reply to Alessio Placitelli [:Dexter] from comment #4)
> (In reply to Shane Caraveo (:mixedpuppy) from comment #3)
> > The only way to fix this is ensure data coming into the addon layer is
> > correct.  This patch would fix the issue.
> 
> I see, this makes sense. Is there any possibility to at least fix the
> Facebook manifest (to reduce the data outage in Telemetry while the fix
> rides the trains)? Is it our of our control (i.e. handled by Facebook)?

only for future installs, there is already a large base.  we should just fix the code.
Attachment #8709553 - Flags: review?(markh)
Attachment #8709553 - Flags: review?(markh) → review+
@Dexter, what firefox versions are affected by this?  Do we need to uplift?
Flags: needinfo?(alessio.placitelli)
(In reply to Shane Caraveo (:mixedpuppy) from comment #7)
> @Dexter, what firefox versions are affected by this?  Do we need to uplift?

Yes please, this is a problem since bug 1211404 landed. We need to uplift this to all channels (where possible).
Flags: needinfo?(alessio.placitelli)
https://hg.mozilla.org/mozilla-central/rev/dacf3116c2de
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
Comment on attachment 8709553 [details] [diff] [review]
ensure string version

Approval Request Comment
[Feature/regressing bug #]: bug 1238142
[User impact if declined]: non, telemetry reporting issue
[Describe test coverage new/current, TreeHerder]: new tests in bug 1240868, existing test coverage in socialapi
[Risks and why]: low, shouldn't have any affect
[String/UUID change made/needed]: none
Attachment #8709553 - Flags: approval-mozilla-aurora?
Comment on attachment 8709553 [details] [diff] [review]
ensure string version

Improve telemetry data, taking it.
Attachment #8709553 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee: nobody → mixedpuppy
Whiteboard: [measurement:client:tracking]
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.