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');

?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dvessel’s picture

Version: 6.2 » 7.x-dev
Status: Active » Needs review
FileSize
662 bytes
650 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

I think this makes sense indeed.

nedjo’s picture

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

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

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

Status: Reviewed & tested by the community » Patch (to be ported)

Patch does not apply in Drupal 5.x.

sun’s picture

Status: Patch (to be ported) » Reviewed & tested by the community
FileSize
526 bytes
drumm’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 5.x.

Anonymous’s picture

Status: Fixed » Closed (fixed)

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