433615 - Firefox should send version numbers to PFS not just build ids

Status: VERIFIED FIXED

(Toolkit Graveyard :: Plugin Finder Service, defect)

Description

[Michael Morgan [:morgamic]]

On minefield I get this when referencing PFS:
?mimetype=application%2Fx-shockwave-flash&appID={ec8030f7-c20a-464f-9b0e-13a3a9e97384}&appVersion=2008051204&clientOS=Intel Mac OS X 10.5&chromeLocale=en-US

I don't think appVersion should be the build id and should instead be the software version.  Given that there is precedent, I suggest we add another URL parameter in Fx3 to designate something as Firefox 3 -- so &version=3.0rc1 for example would be a good start.

This is important as we approach a period where we want PFS to change behavior based on what version the client requesting a plugin is.  If appVersion is a build id, we'll have to start pushing around build ids instead of versions, which is problematic and hard to maintain.

Changing what Flash installer PFS offers based on OS and Firefox version is a good example of a case where the current parameters are insufficient for the server-side behavior we want to implement.

Comment 1

[Mike Connor [:mconnor]]

I don't think this is a showstopper or respin candidate.  We should fix in the next available ship vehicle (whether that's RC2 or 3.0.1) for sure, but unless its absolutely necessary (which it isn't) it can wait.

Comment 2

[Michael Morgan [:morgamic]]

Depends on how important it is to make PFS behave differently for different versions of Firefox.  But I guess we still have build ids -- just that managing sets of build ids vs. matching on version strings is a huge PITA.

Comment 3

[Dave Townsend [:mossop]]

Attachment: patch rev 1

My brain is struggling to find a suitable name for the parameter but gone with appRelease here.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 321045 [details] [diff] [review]
patch rev 1

Is there any benefit to adding this to http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/mozapps/plugins/content/pluginInstallerWizard.js&rev=1.24#546 as well?

Comment 5

[Dave Townsend [:mossop]]

Attachment: patch rev 2

Yes, why not

Comment 6

[Dave Townsend [:mossop]]

Comment on attachment 321528 [details] [diff] [review]
patch rev 2

Seeking approval to land for a potential RC2 or next maintenance release. This is a simple change that provides useful data to the pfs service. Risk is very low

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 321528 [details] [diff] [review]
patch rev 2

a=beltzner, can we get this landed in, uh, short order?

Comment 8

[:Gavin Sharp [email: [e-mail]]]

mozilla/browser/app/profile/firefox.js 	1.337
mozilla/toolkit/mozapps/plugins/content/pluginInstallerDatasource.js 	1.9
mozilla/toolkit/mozapps/plugins/content/pluginInstallerWizard.js 	1.25 

Comment 9

[Dave Townsend [:mossop]]

Attachment: branch patch

This is the same patch made to apply against branch. I believe that we should take this for consistency with trunk.

Comment 10

[Michael Morgan [:morgamic]]

I can see this in 2008052904 on OSX.  Thanks Dave.  Will get working on the server piece.

Comment 11

[Daniel Veditz [:dveditz]]

Comment on attachment 322918 [details] [diff] [review]
branch patch

Approved for 1.8.1.15, a=dveditz for release-drivers

Comment 12

[Dave Townsend [:mossop]]

Landed on branch

Checking in browser/app/profile/firefox.js;
/cvsroot/mozilla/browser/app/profile/firefox.js,v  <--  firefox.js
new revision: 1.71.2.83; previous revision: 1.71.2.82
done
Checking in toolkit/mozapps/plugins/content/pluginInstallerDatasource.js;
/cvsroot/mozilla/toolkit/mozapps/plugins/content/pluginInstallerDatasource.js,v  <--  pluginInstallerDatasource.js
new revision: 1.6.2.2; previous revision: 1.6.2.1
done
Checking in toolkit/mozapps/plugins/content/pluginInstallerWizard.js;
/cvsroot/mozilla/toolkit/mozapps/plugins/content/pluginInstallerWizard.js,v  <--  pluginInstallerWizard.js
new revision: 1.13.2.5; previous revision: 1.13.2.4
done

Comment 13

[Al Billings [:abillings - ex-MoCo]]

Verified for 1.8.1.15. We send "appRelease" now with the version number:

GET https://pfs.mozilla.org/plugins/PluginFinderService.php?mimetype=application%2Fx-java-applet%3Bversion%3D1.4.1&appID={ec8030f7-c20a-464f-9b0e-13a3a9e97384}&appVersion=2008061104&clientOS=Windows%20NT%205.1&chromeLocale=en-US&appRelease=2.0.0.15pre

This was checked with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.15pre) Gecko/2008061104 BonEcho/2.0.0.15pre.

Comment 14

[Al Billings [:abillings - ex-MoCo]]

I also verified on RC2. It shows:

GET https://pfs.mozilla.org/plugins/PluginFinderService.php?mimetype=application%2Fx-java-applet%3Bversion%3D1.4.1&appID={ec8030f7-c20a-464f-9b0e-13a3a9e97384}&appVersion=2008052906&clientOS=Windows%20NT%205.1&chromeLocale=en-US&appRelease=3.0

This is with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9) Gecko/2008052906 Firefox/3.0.