786195 - Refactor JS function to detect Firefox version

Status: VERIFIED FIXED

(www.mozilla.org :: Pages & Content, defect)

Description

[Anthony Ricaud (:rik)]

We're using similar code on a couple of pages to detect if the visitor is on the latest version or not, we should factorise that.

From a quick search, there is at least those pages:
/firefox/speed
/firefox/update
/firefox/fx

Comment 1

[Aishwarya]

Hi, I am Aish I would like to work on this bug. Could you assign it to me?

Comment 2

[Anthony Ricaud (:rik)]

Assigning to Aish. To be clearer, the examples I posted are the URLs of some pages that query that info.

I think a good way to collect all usage is to check for navigator.userAgent.

Comment 3

[Aishwarya]

Attachment: Patch

Comment 4

[Anthony Ricaud (:rik)]

Comment on attachment 661500 [details]
Patch

I'm very sorry for the slow review.

The function only returns the main version number, can you rename the function to getFirefoxMasterVersion to make it clear to anyone using it?

Comment 5

[Aishwarya]

Attachment: Updated Patch - Changed the name of the function detecting the firefox version

Comment 6

[Anthony Ricaud (:rik)]

Comment on attachment 678334 [details] [diff] [review]
Updated Patch - Changed the name of the function detecting the firefox version

Review of attachment 678334 [details] [diff] [review]:
-----------------------------------------------------------------

I should have mentioned it earlier: if you change the function name, please change the name everywhere it is currently used.

Comment 7

[Aishwarya]

Attachment: Patch - Updated : Changed the function names

Apologies for the error. I have made the required changes.

Comment 8

[Anthony Ricaud (:rik)]

Comment on attachment 678396 [details] [diff] [review]
Patch - Updated : Changed the function names

Review of attachment 678396 [details] [diff] [review]:
-----------------------------------------------------------------

r+ thanks!

I somehow typo-ed the name of the function. getFirefoxMajorVersion makes more sense than MasterVersion so I'll commit this with the name changed.

Comment 9

[Anthony Ricaud (:rik)]

Oh actually, could you submit a pull request against https://github.com/mozilla/bedrock/ so that I can credit you properly with the commit?

Comment 10

[[github robot]]

Commit pushed to master at https://github.com/mozilla/bedrock

https://github.com/mozilla/bedrock/commit/dffcebb84c5c928f0c791ab827d7cd36e920be6e
Refactor JS function to detect Firefox version

 fix bug 786195

Comment 11

[Anthony Ricaud (:rik)]

I finally committed it with your Bugzilla info.

Thanks!

Comment 12

[Anthony Ricaud (:rik)]

This has been pushed to production.

Comment 13

[raymond [:retornam] (inactive)]

verified fixed http://www.mozilla.org/en-US/firefox/speed/

http://www.mozilla.org/en-US/firefox/update/

http://www.mozilla.org/en-US/firefox/fx/