1240682 - social addons often have int versions rather than string versions

Status: RESOLVED FIXED

(Firefox Graveyard :: SocialAPI: Providers, defect)

Description

[Alessio Placitelli [:Dexter]]

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"

Comment 1

[Alessio Placitelli [:Dexter]]

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.

Comment 2

[Shane Caraveo (:mixedpuppy)]

Attachment: ensure string version

Comment 3

[Shane Caraveo (:mixedpuppy)]

The only way to fix this is ensure data coming into the addon layer is correct.  This patch would fix the issue.

Comment 4

[Alessio Placitelli [:Dexter]]

(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)?

Comment 5

[Shane Caraveo (:mixedpuppy)]

(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.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/dacf3116c2de

Comment 7

[Shane Caraveo (:mixedpuppy)]

@Dexter, what firefox versions are affected by this?  Do we need to uplift?

Comment 8

[Alessio Placitelli [:Dexter]]

(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).

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 10

[Shane Caraveo (:mixedpuppy)]

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

Comment 11

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8709553 [details] [diff] [review]
ensure string version

Improve telemetry data, taking it.

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1f9e03713e94