768619 - js-class is added to both body and html element

Status: RESOLVED FIXED

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

Description

[Isac Lagerblad (:icaaq)]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:16.0) Gecko/16.0 Firefox/16.0
Build ID: 20120626030539

Steps to reproduce:

1. Press Cmd+N to open a new browser window, then type https://www.mozilla.org/en-US/ in the address bar and press Enter
2. Inspect the element html-element and body-element


Actual results:

Both of them have a js class


Expected results:

Only one of them need a js class

Comment 1

[Craig Cook (:craigcook)]

I think the one on the html element is a remnant from Modernizr. Some templates have class="no-js" hardcoded on the html element, which Modernizr is meant to overwrite with its own classes. But we also have a single line of JavaScript that overwrites that class when JS is enabled even if Modernizr isn't present. The class gets added to the body by the global site.js. True, we don't need both, we just need to decide which one to keep. 

And we should decide if there's a use for the no-js class at all. I tend to assume non-JavaScript is the baseline for styling purposes, then hang added styles from the .js class where needed (progressive enhancement FTW!). That means .no-js just isn't necessary, and also means the non-JavaScript default is less specific than the overriding rule with the .js class (specificity FTW!). We should look through our CSS and see if .no-js being used anywhere.

Comment 2

[Steven Garrity [:sgarrity]]

Good catch Isac.

Looking at at the base template (https://github.com/mozilla/bedrock/blob/master/templates/base.html), we're adding the 'js' class in two places (once of the root element, and once on the body class), as you suggest.

It was added to the HTML element in the in-line JS in the base.html and base-resp.html templates, and added to the body in the /media/js/site.js file included in those templates.

I've consoloted them and created a pull request. Please review: https://github.com/mozilla/bedrock/pull/207

Comment 3

[Isac Lagerblad (:icaaq)]

Looks good :)

Comment 4

[Steven Garrity [:sgarrity]]

Merged to master.

Comment 5

[[github robot]]

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

https://github.com/mozilla/bedrock/commit/d3faa2cda2b2a22c213d055c5e6e4efcd05a41a0
Consolodate duplicate 'no-js' class management
(Bug 768619)