562372 - Drop YUI

Status: VERIFIED FIXED

(www.mozilla.org :: General, defect)

Description

[Alex Buchanan [:abuchanan]]

For tracking progress on removing YUI from mozilla.com.

There's lots to rewrite:

[abuchanan@khan js]$ grep -l YAHOO *
careers.js
detect-flash.js
firefox-landing.js
firefox-video-launch.js
firstrun.js
input-placeholder.js
lang-pack.js
mobile-desktop.js
mozilla-all-1.js
mozilla-all-2.js
mozilla-customize-page.js
mozilla-expanders.js
mozilla-get-personal-page.js
mozilla-language-search.js
mozilla-map.js
mozilla-menu.js
mozilla-pager.js
mozilla-start-page.js
mozilla-video-player.js
mozilla-video-tools.js
whats-new-page.js

Comment 1

[Steven Garrity [:sgarrity]]

Can you elaborate a bit on the rationale for this? Will we be able to achieve the desired effects with better performance without using YUI?

Comment 2

[Alex Buchanan [:abuchanan]]

It's more about consistency than performance.  We have both YUI and jQuery on the site currently, and I'd like to use only one.  jQuery seems to be the flavor of choice for mozilla sites in general, and I think we should follow suit.

That said, there may be slight performance improvements, including not loading both jquery and YUI on some pages.

This is certainly not a priority, but it's something I'd like to work towards, so filed this bug to track that.

Are there objections to removing YUI?

Comment 3

[Michael Gauthier [:gauthierm]]

For Mozilla.com, Mozilla Messaging and Mozilla Support the majority of the code is using YUI, not jQuery.

I think jQuery was added as a dependency to Mozilla.com for the plugin finder page. The geolocation demo page also uses jQuery. I'm not sure why jQuery is included on every page when it is only used on these two pages. I'm not sure why those pages used jQuery instead of YUI when YUI was already included.

It will be a large amount of work to port all the existing code to jQuery. Some parts will be easy to port (like the video player), others will be more complicated (the tabbed-pager and the the main menu).

I see the point in moving to one JavaScript library for a site, but I'm not sure choosing jQuery for Mozilla.com is the right choice.

Comment 4

[Alex Buchanan [:abuchanan]]

(In reply to comment #3)
> For Mozilla.com, Mozilla Messaging and Mozilla Support the majority of the code
> is using YUI, not jQuery.

True except for SUMO.  AIUI, Support only uses it for the header nav.  Generally, they use jQuery.

> I think jQuery was added as a dependency to Mozilla.com for the plugin finder
> page. The geolocation demo page also uses jQuery. I'm not sure why jQuery is
> included on every page when it is only used on these two pages. I'm not sure
> why those pages used jQuery instead of YUI when YUI was already included.

Good point, I'll work on a short term fix of only loading that where it's necessary.

> It will be a large amount of work to port all the existing code to jQuery. Some
> parts will be easy to port (like the video player), others will be more
> complicated (the tabbed-pager and the the main menu).

The main menu is already mostly done as part of bug 557562, and it wasn't a ton of work.  I haven't looked at all the libraries yet, so I don't know how tedious this will be.  Some aren't needed / used, e.g. careers.js.  

Personally, I don't mind doing this extra work, especially since it's a low priority enhancement.
 
> I see the point in moving to one JavaScript library for a site, but I'm not
> sure choosing jQuery for Mozilla.com is the right choice.

Yes, I was being presumptive by not expecting any preference for YUI.  Apologies.

Most important to me is that it seems jQuery is the library of choice for Mozilla Webdev in general.

Otherwise, personally I like jQuery for it's simplicity, brevity, general popularity.  I've found jQuery easier to learn, and accomplishes tasks with what feels like simple, concise code.  I get the opposite from YUI.  I admit, that's all qualitative, not quantitative ;)

I should note, the existing mozilla-* YUI libraries work well and have been great to us.  If we can get the same effect with a better foundation from jQuery, let's go for it.  If there's a strong preference to keeping YUI, I'm open to dropping this.

Comment 5

[Michael Gauthier [:gauthierm]]

(In reply to comment #4)
> (In reply to comment #3)
> > For Mozilla.com, Mozilla Messaging and Mozilla Support the majority of the code
> > is using YUI, not jQuery.
> 
> True except for SUMO.  AIUI, Support only uses it for the header nav. 
> Generally, they use jQuery.
> 
> > I think jQuery was added as a dependency to Mozilla.com for the plugin finder
> > page. The geolocation demo page also uses jQuery. I'm not sure why jQuery is
> > included on every page when it is only used on these two pages. I'm not sure
> > why those pages used jQuery instead of YUI when YUI was already included.
> 
> Good point, I'll work on a short term fix of only loading that where it's
> necessary.
> 

Awesome.

> > It will be a large amount of work to port all the existing code to jQuery. Some
> > parts will be easy to port (like the video player), others will be more
> > complicated (the tabbed-pager and the the main menu).
> 
> The main menu is already mostly done as part of bug 557562, and it wasn't a ton
> of work.  I haven't looked at all the libraries yet, so I don't know how
> tedious this will be.  Some aren't needed / used, e.g. careers.js.  
> 
> Personally, I don't mind doing this extra work, especially since it's a low
> priority enhancement.

That is promising. I'm glad to hear that it won't be as much work as I anticipated.

> > I see the point in moving to one JavaScript library for a site, but I'm not
> > sure choosing jQuery for Mozilla.com is the right choice.
> 
> Yes, I was being presumptive by not expecting any preference for YUI. 
> Apologies.

I like both jQuery and YUI; my preference for Mozilla.com is based on the existing code being written in YUI. That said, if it's going to be relatively easy to port the code to jQuery, my preference for one framework instead of two will trump that opinion.

> Most important to me is that it seems jQuery is the library of choice for
> Mozilla Webdev in general.
> 
> Otherwise, personally I like jQuery for it's simplicity, brevity, general
> popularity.  I've found jQuery easier to learn, and accomplishes tasks with
> what feels like simple, concise code.  I get the opposite from YUI.  I admit,
> that's all qualitative, not quantitative ;)
> 
> I should note, the existing mozilla-* YUI libraries work well and have been
> great to us.  If we can get the same effect with a better foundation from
> jQuery, let's go for it.  If there's a strong preference to keeping YUI, I'm
> open to dropping this.

Both libraries has their strengths and weaknesses. I don't have a strong preference for either framework (though I do have more experience with YUI), so keep this bug open. Where possible, I'll use jQuery for new code so as to reduce the porting workload down the road.

Comment 6

[Kohei]

We have a jQuery version of mozilla-customize-page.js at
http://mozilla.jp/js/mozilla-customize-page.js

Note that the current plugincheck.js doesn't work with jQuery 1.4.x, we still use 1.3.2. I'm working on modifying plugincheck.js to support the latest jQuery.

Comment 7

[Kohei]

(In reply to comment #6)
> We have a jQuery version of mozilla-customize-page.js at
> http://mozilla.jp/js/mozilla-customize-page.js

Oh, the live page is
http://mozilla.jp/firefox/customize/

Comment 8

[Alex Buchanan [:abuchanan]]

I grep'd the tree and nothing refers to this file,
http://svn.mozilla.org/projects/mozilla.com/trunk/js/mozilla-video-player.js

Comment 9

[Steven Garrity [:sgarrity]]

Alex: confirmed, mozilla-video-player.js has been replaced by mozilla-video-tools.js.

Comment 10

[Steven Garrity [:sgarrity]]

Deleting       js/mozilla-video-player.js
Committed revision 67047.

Comment 11

[Michael Gauthier [:gauthierm]]

As part of the first-run page work, I ported the expanders to jQuery. See r67241.

Comment 12

[Steven Garrity [:sgarrity]]

Filed bug 566319 to track getting the input-placeholder.js script replaced with the jquery version.

Comment 13

[Steven Garrity [:sgarrity]]

We've chipped away at the list of YUI-based scripts a bit.

careers.js is no longer used (removed)
input-placeholder.js has been re-written as mozilla-input-placeholder.js without YUI
mozilla-expanders.js has been re-written without YUI (will need to ensure that the many pages that use it include jquery-min
mozilla-video-player.js was no longer used (removed).

Once we get the mozilla-expanders.js pushed to live, the firstrun/whatsnew pages can drop YUI JS.

This leaves:

detect-flash.js
firefox-landing.js
firefox-video-launch.js
firstrun.js
lang-pack.js
mobile-desktop.js
mozilla-all-1.js
mozilla-all-2.js
mozilla-customize-page.js
mozilla-get-personal-page.js
mozilla-language-search.js
mozilla-map.js
mozilla-menu.js
mozilla-pager.js
mozilla-start-page.js
mozilla-video-tools.js
whats-new-page.js

Comment 14

[Steven Garrity [:sgarrity]]

mozilla-all-1.js and mozilla-all-2.js removed in r67350 (they were from expired tests).

Down to:

detect-flash.js
firefox-landing.js
firefox-video-launch.js
firstrun.js
lang-pack.js
mobile-desktop.js
mozilla-customize-page.js
mozilla-get-personal-page.js
mozilla-language-search.js
mozilla-map.js
mozilla-menu.js
mozilla-pager.js
mozilla-start-page.js
mozilla-video-tools.js
whats-new-page.js

Comment 15

[Steven Garrity [:sgarrity]]

removed firefox-landing.js and firefox-video-launch.js in r67351 (both unused).

Down to:

detect-flash.js
firstrun.js
lang-pack.js
mobile-desktop.js
mozilla-customize-page.js
mozilla-get-personal-page.js
mozilla-language-search.js
mozilla-map.js
mozilla-menu.js
mozilla-pager.js
mozilla-start-page.js
mozilla-video-tools.js
whats-new-page.js

Comment 16

[Michael Gauthier [:gauthierm]]

lang-pack.js was converted in r70383 in response to Bug 576679.

Comment 17

[Steven Garrity [:sgarrity]]

Down to:

detect-flash.js
firstrun.js
mobile-desktop.js
mozilla-customize-page.js
mozilla-get-personal-page.js
mozilla-language-search.js
mozilla-map.js
mozilla-menu.js
mozilla-pager.js
mozilla-start-page.js
mozilla-video-tools.js
whats-new-page.js

Comment 18

[Alex Buchanan [:abuchanan]]

bug 538486 will drop all pages using firstrun.js, so that makes the list,

detect-flash.js
mobile-desktop.js
mozilla-customize-page.js
mozilla-get-personal-page.js
mozilla-language-search.js
mozilla-map.js
mozilla-menu.js
mozilla-pager.js
mozilla-start-page.js
mozilla-video-tools.js
whats-new-page.js

Comment 19

[Pascal Chevrel:pascalc]

add to this list mozilla-video-tools-l10n.js please, we use a slightly different file for locales that allows subtitling.

Comment 20

[Alex Buchanan [:abuchanan]]

(In reply to comment #19)
> add to this list mozilla-video-tools-l10n.js please, we use a slightly
> different file for locales that allows subtitling.

we should merge the two, making the list:

detect-flash.js
mobile-desktop.js
mozilla-customize-page.js
mozilla-get-personal-page.js
mozilla-language-search.js
mozilla-map.js
mozilla-menu.js
mozilla-pager.js
mozilla-start-page.js
mozilla-video-tools.js + mozilla-video-tools-l10n.js
whats-new-page.js

Comment 21

[Alex Buchanan [:abuchanan]]

bug 557562 will drop the need for mozilla-menu.js, so updated list is...

detect-flash.js
mobile-desktop.js
mozilla-customize-page.js
mozilla-get-personal-page.js
mozilla-language-search.js
mozilla-map.js
mozilla-pager.js
mozilla-start-page.js
mozilla-video-tools.js + mozilla-video-tools-l10n.js
whats-new-page.js

Comment 22

[Michael Gauthier [:gauthierm]]

Mozilla Pager was converted in r73520. One page that relied on external YUI interfaces was updated in r73520. Updated list is:

detect-flash.js
mobile-desktop.js
mozilla-customize-page.js
mozilla-get-personal-page.js
mozilla-language-search.js
mozilla-map.js
mozilla-start-page.js
mozilla-video-tools.js + mozilla-video-tools-l10n.js
whats-new-page.js

Comment 23

[Kohei]

Here's the jQuery version of mozilla-customize-page.js.
Feel free to use http://mozilla.jp/js/mozilla-customize-page.js

Comment 24

[Steven Garrity [:sgarrity]]

All pages using mozilla-pager.js are now safely including jquery (trunk r74425). The jquery-based mozilla-expanders.js could probably go to production now.

Comment 25

[Steven Garrity [:sgarrity]]

detect-flash.js was de-YUIfied by aricaud in r76495. The new list:

mobile-desktop.js
mozilla-customize-page.js
mozilla-get-personal-page.js
mozilla-language-search.js
mozilla-map.js
mozilla-start-page.js
mozilla-video-tools.js + mozilla-video-tools-l10n.js
whats-new-page.js

Comment 26

[Kohei]

Attachment: jQueried mozilla-video-tools.js

Removed VideoControl and VideoScaler as they're not used.

Note: you have to remove 
  Mozilla.VideoScaler.close_text = '{$l10n->get('Close')}';
in some /includes/ files to avoid errors.

Comment 27

[Kohei]

Attachment: jQueried mozilla-video-tools-addsubtitles.js

Comment 28

[Kohei]

Comment on attachment 490043 [details]
jQueried mozilla-video-tools.js

Ah, VideoScaler is used on the Firefox 3.5 firstrun pages...

Comment 29

[Kohei]

Attachment: jQueried mozilla-video-tools.js, v2

Here we go. This script assumes the CSS margin of .mozilla-video-control is 0.
.mozilla-video-scaler can have the margin instead.

Comment 30

[Kohei]

Attachment: patch for firstrun-page-3-5.css

> This script assumes the CSS margin of .mozilla-video-control is 0.
> .mozilla-video-scaler can have the margin instead.

Here's the patch.

Comment 31

[Anthony Ricaud (:rik)]

Deleted mozilla-get-personal-page.js in 82031 and mozilla-menu.js and mozilla-start-page.js in r82038

mobile-desktop.js
mozilla-customize-page.js
mozilla-language-search.js
mozilla-map.js
mozilla-video-tools.js
whats-new-page.js

Comment 32

[Anthony Ricaud (:rik)]

r82039 on stage

Comment 33

[Anthony Ricaud (:rik)]

Committed r82044 on trunk and r82046 on stage.

So we only have
mozilla-language-search.js
mozilla-map.js
mozilla-video-tools.js
left.

And Kohei already submitted a patch for mozilla-video-tools so that's looking good!

Comment 34

[Laura Forrest]

Hey Anthony - who should work on this now? Please update and assign to someone. Thanks!

Comment 35

[Anthony Ricaud (:rik)]

I'm still on it, I unassigned it by mistake.

Comment 36

[Anthony Ricaud (:rik)]

Committed r82633.

Now let's test mozilla-video-tools.js.

Comment 37

[Anthony Ricaud (:rik)]

Committed r82717.

Thanks a lot Kohei for this one! I just had to fix one or two things for IE from your patch.

And so this is done! We'll need to watch out for regressions and also remove all calls to YUI.

Comment 38

[Anthony Ricaud (:rik)]

Committed r82723.

Remove some inline calls.

Comment 39

[Anthony Ricaud (:rik)]

Committed r82892 on stage.

James, I'm using the whiteboard to list all revisions in stage that will need to be merged into production.

Comment 40

[Anthony Ricaud (:rik)]

Committed r82898 on stage.

This is a merge for r80046,r64039,r67304,r67351,r67295,r70383,r67350 from trunk.

Comment 41

[Anthony Ricaud (:rik)]

OK so marking as QA needed although I'm not quite sure how to test all this. If there is any new JS error on http://www.stage.mozilla.com that should be my fault.

I've filed bug 635269 to remove the inclusions of YUI.

Comment 42

[raymond [:retornam] (inactive)]

qa-verified-stage http://www.stage.mozilla.com/en-US/firefox/ no JS errors from my testing  last week and today

Comment 43

[James Long (:jlongster)]

pushed to production r83056

Comment 44

[raymond [:retornam] (inactive)]

verified fixed http://www.mozilla.com/