We just had an issue with Drupal and sIFR--it turned out that sIFR was adding class "sIFR-active" to the html element, but then drupal.js was stomping on it. Would it not make sense to replace:

document.documentElement.className = 'js';

with

$(document.documentElement).addClass('js');

?

CommentFileSizeAuthor
#7 drupal-DRUPAL-5.html-js.patch526 bytessun
#1 js_class_add_not_overwrite_head.patch650 bytesdvessel
#1 js_class_add_not_overwrite_6.patch662 bytesdvessel

Comments

dvessel’s picture

dvessel commented
Version: 6.2 » 7.x-dev
Status: Active » Needs review
StatusFileSize
new js_class_add_not_overwrite_6.patch662 bytes
new js_class_add_not_overwrite_head.patch650 bytes

Makes sense to me. Brief testing brought no issues. Shouldn't have any side-effects.

Marking for Head. Patch supplied for both 6/head.

dries’s picture

dries commented

I think this makes sense indeed.

nedjo’s picture

nedjo
he/him/his
Primary language English
commented

Yes, a simple and appropriate improvement. Not marking RTBC because I haven't tested, though arguably a change this simple doesn't need testing.

dries’s picture

dries commented
Version: 7.x-dev » 6.x-dev
Status: Needs review » Reviewed & tested by the community

I've committed this to CVS HEAD because I think it is the right thing to do. I'll let Gabor review the Drupal 6 version of the patch just to make sure we had a couple more eyes look at it. :-)

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Version: 6.x-dev » 5.x-dev

Looks right. I think this code predates jQuery and was not properly updated yet. Also, it probably did not surface yet, because the HTML root tag is not commonly used to class the page (the body is much more often used). Committed to 6.x.

I looked at the D5 code, and it also looks like this should be fixed there, so moving to Drupal 5.

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Reviewed & tested by the community » Patch (to be ported)

Patch does not apply in Drupal 5.x.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Patch (to be ported) » Reviewed & tested by the community
StatusFileSize
new drupal-DRUPAL-5.html-js.patch526 bytes
drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Reviewed & tested by the community » Fixed

Committed to 5.x.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.