918260 - Update bedrock jQuery to version 1.11

Status: RESOLVED FIXED

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

Description

[Alex Gibson [:agibson]]

Bedrock is currently using version version 1.7.1 of jQuery, which is getting a bit old - we should look to update it to 1.10.

Looking at pages & content, the main obstacle preventing bedrock using 1.10 are pages/plugins using `$.browser`, which was removed in jQuery 1.9.

We should remove use of `$.browser` on bedrock wherever possible. Most pages just seem to be looking for old IE versions. 

For 3rd party plugins that use it, we should see if updated versions are available. If an update is not available, we can use the jQuery Migrate[1] plugin that restores `$.browser` functionality on those pages.

We could probably handle this bug by updating bedrock pages/content in sections.

[1] https://github.com/jquery/jquery-migrate/#readme

Comment 1

[Jennifer Bertsch [:jbertsch]]

Hi Alex, Pmac, and Mike-

Let's talk about this at our next mozilla.org weekly status meeting.

Thx,
Jen

Comment 2

[Eduardo Veiga]

Hi, I would like to work on this, are there currently client side tests to check we don't break anything?
Also, what would you recommend to do with the current usages of $.browser? The jQuery upgrade guide suggests Morenizr, is that ok? It looks like lots of other methods where removed from the API in 1.9, we should check for all those too, right?

Comment 3

[Alex Gibson [:agibson]]

(In reply to Eduardo Veiga from comment #2)
> Hi, I would like to work on this, are there currently client side tests to
> check we don't break anything?
> Also, what would you recommend to do with the current usages of $.browser?
> The jQuery upgrade guide suggests Morenizr, is that ok? It looks like lots
> of other methods where removed from the API in 1.9, we should check for all
> those too, right?

Hi Eduardo,

Thanks for showing interest in working on this bug!

On bedrock we only have a certain number of client-side tests currently, and these mainly cover areas related to download buttons and general helper plugins that are specific to our code base. The majority of page-specific JavaScript does not have client side tests currently, and will therefore need manual testing and verification.

There are many pages on bedrock, so if you are comfortable taking on some of this work it may be best to split this work up into a few different pull requests. Perhaps a different PR for each main section of the site? This would likely also make code review and QA verification a bit easier.

You are correct when you say some API methods we're removed in jQuery 1.9, so there may be pages that require some minor updates to the JS. As for the usage of $.browser, we should try to remove usage of it where ever possible by using feature detection. On pages where we are just checking for a browser version (e.g. to show a piece of conditional information specific to IE), we should replace it with our own UA check.

Hope that helps!

Comment 4

[Eduardo Veiga]

Hi Alex, thanks for all the clarifications!

I like the idea of splitting this into parts, I'm really new to the bedrock codebase, what sections of the code do you suggest? maybe by each django app?

Didn't know about the UA checker, where can I get some docs about it (or its code)?

Comment 5

[Alex Gibson [:agibson]]

(In reply to Eduardo Veiga from comment #4)
> what sections of the code do you suggest? maybe by each django app?

I'd say that's probably a good way forward :)

> Didn't know about the UA checker, where can I get some docs about it (or its
> code)?

I just mean replacing things like `if ($.browser.msie && $.browser.version < 9)` with our own UA check using a regular expression.

> I'm really new to the bedrock codebase

I suggest checking out the bedrock documentation here: http://bedrock.readthedocs.org/en/latest/. It has info on getting set up for local development, coding style guidelines and Git tips for our workflow.

Good luck!

Comment 6

[Eduardo Veiga]

I started with the update for each django app, and started with mozorg.
I tested all of its views using the jQuery migrate plugin to check for deprecated usages.
This part is done, but would like someone to review the user agent detection code, the diff is available here:
https://github.com/cinemascop89/bedrock/compare/918260-jquery-update

Comment 7

[Alex Gibson [:agibson]]

Hi Eduardo, this looks great thank you!

The only thing I would say we probably don't need here is the full-blown UA detection code. `$.browser` was removed from jQuery to discourage people relying on it and to use feature detection. On bedrock, we should try and do the same. All we really need here is a single check for IE6 in one of our video libs. I'd say in this instance it would be fine just to replace that one line of code with a regexp.

Comment 8

[Alex Gibson [:agibson]]

Also, since this bug was created I believe jQuery version 1.11 has been released. Worth jumping straight to it if there are not many major changes since v1.10?

Comment 9

[Eduardo Veiga]

Hi Alex, 
I removed all that ua detection code, also changed the jQuery version to 1.11, the release info says they made no breaking changes so it's safe.

Pull request here: https://github.com/mozilla/bedrock/pull/1651

Comment 10

[Alex Gibson [:agibson]]

Thanks, Eduardo! I will get round to reviewing your PR shortly.

Comment 11

[[github robot]]

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

https://github.com/mozilla/bedrock/commit/829c827d1e8c12862da8d2e2c06ba636729da1ac
Bug 918260 - Update jQuery to 1.10.1 in mozorg

https://github.com/mozilla/bedrock/commit/66c50e97dd8faca4369991d011f13ea1e5a06b9e
Merge pull request #1651 from cinemascop89/918260-jquery-update

Bug 918260 - Update jQuery to 1.11.0 in mozorg

Comment 12

[Alex Gibson [:agibson]]

Eduardo has successfully updated the bedrock pages using the 'mozorg-resp' JS bundle to use the latest jQuery. This includes areas such as the homepage, /about, /contact/, /contribute, /mission and other sections of the site.

Let's keep this bug open, as there are still other bundles to update. Next it's probably best to do the Firefox app, so pages using the following bundles:

'firefox'
'firefox-resp'

Last of all let's do the common bundles in the base templates:

'common'
'common-resp'

Comment 13

[Alex Gibson [:agibson]]

Attachment: GitHub pull request