Odd (minor) javascript error

Started by Antechinus, January 15, 2015, 11:12:20 PM

Previous topic - Next topic

Antechinus

Got a strange one here. It doesn't appear to break anything, but since it's the only js error I have it'd be nice to nail it.
The line of code that's generating this error is this one:

// Add a load event for the function above.
addLoadEvent(smc_toggleImageDimensions);


Which is just after the end of this function in theme.js

// Toggles the element height and width styles of an image.
function smc_toggleImageDimensions()
{
var oImages = document.getElementsByTagName('IMG');
for (oImage in oImages)
{
// Not a resized image? Skip it.
if (oImages[oImage].className == undefined || oImages[oImage].className.indexOf('bbc_img resized') == -1)
continue;

oImages[oImage].style.cursor = 'pointer';
oImages[oImage].onclick = function() {
this.style.width = this.style.height = this.style.width == 'auto' ? null : 'auto';
};
}
}


Now the weird thing is that even if this code is removed, the attachment thumbnails still expand and contract on click, like they ought to. So the code doesn't actually appear to have anything to do with changing image dimensions, AFAICT, or at least not the way Apocalypse is set up.

If this code is removed, the js error goes away and everything is cool. So the big questions are: what is this code doing in theme.js? What's its purpose? Am I missing something that will break if I remove this code?

Arantor

You need to load script.js before theme.js and apparently your hoverintent file too.

To elaborate: the same function is defined in both your minified code and in theme.js, but script.js is not loaded early enough for one of them to work correctly (since addLoadEvent is defined in there)
Holder of controversial views, all of which my own.


Antechinus

Aha. Got it. So what does the code do though, since my images are expanding and contracting on click anyway?

Arantor

See my edit... it works because you have the same code in there twice, only the dependency of script.js isn't available when it's first called.
Holder of controversial views, all of which my own.


Antechinus

Bugger. Now I'll have to find where else theme.js is being called. This theme doesn't have one, so it must be calling from default somewhere.

Arantor

So I looked again. My bad, theme.js isn't loaded, it's minified into your other file.

It still works because it's called in specific cases, but your problem is not theme.js. Move your script.js call BEFORE your other stuff. That will fix the error.
Holder of controversial views, all of which my own.


Antechinus

Ok, but why do the images still toggle dimensions even if I remove that code from the minified file? No function at all left, anywhere. Images still expand and contract on click. Even if cache has just been cleared, so old file can't be hanging in browser.

Arantor

I honestly don't know what you've done without seeing it right there.

What I do know is that the code was in theme.js, was copied to the minified file and is triggered from loading the minified file, and failing because it's missing a dependency in script.js. If that behaviour continues to work, either you didn't remove it properly or you have something *else* doing that work for you.
Holder of controversial views, all of which my own.


Antechinus

Ok, here's the file that is being loaded. It's only line 6 that is relevant, since hoverIntent and Superclick have nothing to do with SMF attachments.

As you can see the function in question is completely commented out, just for testing now. The only active bit of the old theme.js is function smf_addButton and the rest has been stripped out.

/*hoverIntent r7 2013.03.11 jQuery 1.9.1+ Copyright 2007, 2013 Brian Cherne. You may use hoverIntent under the terms of the MIT license. http://cherne.net/brian/resources/jquery.hoverIntent.html*/
(function($){$.fn.hoverIntent=function(handlerIn,handlerOut,selector){var cfg={interval:50,sensitivity:20,timeout:0};if(typeof handlerIn==="object"){cfg=$.extend(cfg,handlerIn)}else if($.isFunction(handlerOut)){cfg=$.extend(cfg,{over:handlerIn,out:handlerOut,selector:selector})}else{cfg=$.extend(cfg,{over:handlerIn,out:handlerIn,selector:handlerOut})}var cX,cY,pX,pY;var track=function(ev){cX=ev.pageX;cY=ev.pageY};var compare=function(ev,ob){ob.hoverIntent_t=clearTimeout(ob.hoverIntent_t);if((Math.abs(pX-cX)+Math.abs(pY-cY))<cfg.sensitivity){$(ob).off("mousemove.hoverIntent",track);ob.hoverIntent_s=1;return cfg.over.apply(ob,[ev])}else{pX=cX;pY=cY;ob.hoverIntent_t=setTimeout(function(){compare(ev,ob)},cfg.interval)}};var delay=function(ev,ob){ob.hoverIntent_t=clearTimeout(ob.hoverIntent_t);ob.hoverIntent_s=0;return cfg.out.apply(ob,[ev])};var handleHover=function(e){var ev=jQuery.extend({},e);var ob=this;if(ob.hoverIntent_t){ob.hoverIntent_t=clearTimeout(ob.hoverIntent_t)}if(e.type=="mouseenter"){pX=ev.pageX;pY=ev.pageY;$(ob).on("mousemove.hoverIntent",track);if(ob.hoverIntent_s!=1){ob.hoverIntent_t=setTimeout(function(){compare(ev,ob)},cfg.interval)}}else{$(ob).off("mousemove.hoverIntent",track);if(ob.hoverIntent_s==1){ob.hoverIntent_t=setTimeout(function(){delay(ev,ob)},cfg.timeout)}}};return this.on({'mouseenter.hoverIntent':handleHover,'mouseleave.hoverIntent':handleHover},cfg.selector)}})(jQuery);
/*jQuery Superclick Menu Plugin v1.1.0 Copyright (c) 2014 Joel Birch. Dual licensed under the MIT and GPL licenses: http://www.opensource.org/licenses/mit-license.php - http://www.gnu.org/licenses/gpl.html*/
;(function(t){"use strict";var s=function(){var s={bcClass:"sf-breadcrumb",menuClass:"sf-js-enabled",anchorClass:"sf-with-ul",menuArrowClass:"sf-arrows"},e=(function(){t(window).load(function(){t("body").children().on("click.superclick",function(){var s=t(".sf-js-enabled");s.superclick("reset")})})}(),function(t,e){var n=s.menuClass;e.cssArrows&&(n+=" "+s.menuArrowClass),t.toggleClass(n)}),n=function(e,n){return e.find("li."+n.pathClass).slice(0,n.pathLevels).addClass(n.activeClass+" "+s.bcClass).filter(function(){return t(this).children(n.popUpSelector).hide().show().length}).removeClass(n.pathClass)},i=function(t){t.children("a").toggleClass(s.anchorClass)},a=function(t){var s=t.css("ms-touch-action");s="pan-y"===s?"auto":"pan-y",t.css("ms-touch-action",s)},o=function(s){var e,n=t(this),i=n.siblings(s.data.popUpSelector);return i.length?(e=i.is(":hidden")?r:c,t.proxy(e,n.parent("li"))(),!1):void 0},r=function(){var s=t(this);h(s),s.siblings().superclick("hide").end().superclick("show")},c=function(){var s=t(this),e=h(s);t.proxy(l,s,e)()},l=function(s){s.retainPath=t.inArray(this[0],s.$path)>-1,this.superclick("hide"),this.parents("."+s.activeClass).length||(s.onIdle.call(p(this)),s.$path.length&&t.proxy(r,s.$path)())},p=function(t){return t.closest("."+s.menuClass)},h=function(t){return p(t).data("sf-options")};return{hide:function(s){if(this.length){var e=this,n=h(e);if(!n)return this;var i=n.retainPath===!0?n.$path:"",a=e.find("li."+n.activeClass).add(this).not(i).removeClass(n.activeClass).children(n.popUpSelector),o=n.speedOut;s&&(a.show(),o=0),n.retainPath=!1,n.onBeforeHide.call(a),a.stop(!0,!0).animate(n.animationOut,o,function(){var s=t(this);n.onHide.call(s)})}return this},show:function(){var t=h(this);if(!t)return this;var s=this.addClass(t.activeClass),e=s.children(t.popUpSelector);return t.onBeforeShow.call(e),e.stop(!0,!0).animate(t.animation,t.speed,function(){t.onShow.call(e)}),this},destroy:function(){return this.each(function(){var n,o=t(this),r=o.data("sf-options");return r?(n=o.find(r.popUpSelector).parent("li"),e(o,r),i(n),a(o),o.off(".superclick"),n.children(r.popUpSelector).attr("style",function(t,s){return s.replace(/display[^;]+;?/g,"")}),r.$path.removeClass(r.activeClass+" "+s.bcClass).addClass(r.pathClass),o.find("."+r.activeClass).removeClass(r.activeClass),r.onDestroy.call(o),o.removeData("sf-options"),void 0):!1})},reset:function(){return this.each(function(){var s=t(this),e=h(s),n=t(s.find("."+e.activeClass).toArray().reverse());n.children("a").trigger("click")})},init:function(r){return this.each(function(){var c=t(this);if(c.data("sf-options"))return!1;var l=t.extend({},t.fn.superclick.defaults,r),p=c.find(l.popUpSelector).parent("li");l.$path=n(c,l),c.data("sf-options",l),e(c,l),i(p),a(c),c.on("click.superclick","a",l,o),p.not("."+s.bcClass).superclick("hide",!0),l.onInit.call(this)})}}}();t.fn.superclick=function(e){return s[e]?s[e].apply(this,Array.prototype.slice.call(arguments,1)):"object"!=typeof e&&e?t.error("Method "+e+" does not exist on jQuery.fn.superclick"):s.init.apply(this,arguments)},t.fn.superclick.defaults={popUpSelector:"ul,.sf-mega",activeClass:"sfHover",pathClass:"overrideThisToUse",pathLevels:1,animation:{opacity:"show"},animationOut:{opacity:"hide"},speed:"normal",speedOut:"fast",cssArrows:!0,onInit:t.noop,onBeforeShow:t.noop,onShow:t.noop,onBeforeHide:t.noop,onHide:t.noop,onIdle:t.noop,onDestroy:t.noop}})(jQuery);
/*SMF theme.js */
/*function smc_toggleImageDimensions(){var oImages=document.getElementsByTagName('IMG');for(oImage in oImages){if(oImages[oImage].className==undefined||oImages[oImage].className.indexOf('bbc_img resized')==-1)continue;oImages[oImage].style.cursor='pointer';oImages[oImage].onclick=function(){this.style.width=this.style.height=this.style.width=='auto'?null:'auto'}}}addLoadEvent(smc_toggleImageDimensions);*/function smf_addButton(sButtonStripId,bUseImage,oOptions){var oButtonStrip=document.getElementById(sButtonStripId);var aItems=oButtonStrip.getElementsByTagName('span');if(aItems.length>0){var oLastSpan=aItems[aItems.length-1];oLastSpan.className=oLastSpan.className.replace(/\s*last/,'position_holder')}var oButtonStripList=oButtonStrip.getElementsByTagName('ul')[0];var oNewButton=document.createElement('li');setInnerHTML(oNewButton,'<a href="'+oOptions.sUrl+'" '+('sCustom'in oOptions?oOptions.sCustom:'')+'><span class="last"'+('sId'in oOptions?' id="'+oOptions.sId+'"':'')+'>'+oOptions.sText+'</span></a>');oButtonStripList.appendChild(oNewButton)}

Antechinus

Note that I'll still put script.js first anyway. I'm not arguing about that. I'm just baffled that the only javascript I can find that mentions toggling image size doesn't appear to do anything, since image size still toggles just fine without it.

Arantor

I actually meant to see the whole thing in situ, as in to be able to examine what's actually happening on those items being clicked and what events are firing - and what function is handling them.

If it's still working when you comment out that (and presumably flush it in the browser cache, hard reset is not always enough), something else is doing the work instead. Maybe there's a duplicate lurking somewhere else...
Holder of controversial views, all of which my own.


Antechinus

Yeah it's an odd one. I've run recursive searches more than once for various terms, including all instances of "toggle" from root directory down, and can't find anything that would be doing that job. Plenty of toggles for collapse/expand of various divs, etc, but that's different code entirely.

And yes I've been emptying both browser cache and forum file cache. It's a weird one. I think I'll just leave it commented out for now and see how it goes.

Antechinus

Aha! I bet it's this little bugger at the end of topic.js.

// *** Other functions...
function expandThumb(thumbID)
{
var img = document.getElementById('thumb_' + thumbID);
var link = document.getElementById('link_' + thumbID);
var tmp = img.src;
img.src = link.href;
link.href = tmp;
img.style.width = '';
img.style.height = '';
return false;
}


ETA: Yup. It is.

Arantor

Holder of controversial views, all of which my own.


Antechinus

Ok I have it figured out. That first function has nothing to do with attachment thumbnails. It's for expanding images that have been resized within posts, to fit the limits imposed in admin for linked-in images. Since I'm running with those limits disabled, and resizing such images with CSS instead, I couldn't see a damned bit of difference anywhere when I removed that function. With the CSS I'm using that function is basically deprecated.

This makes sense now. :)

ETA: Anyway this means I can ditch it and save some js bytes, but I'll still load script.js first. Might as well.

The other thing is that I just realised it's pointless loading hoverIntent with click-actuated menus. It's only worthwhile with Superfish, not with Superclick, so I can save some more js there too. :D

Arantor

Yup, that's it. Nicely figured out :)
Holder of controversial views, all of which my own.


Antechinus

Actually, on second thought what I'll do is leave the load order in the template the same. Since I have to edit the minified files anyway, I'll just completely remove all SMF theme.js remnants from them and append the minified function smf_addButton() to the end of script.js.

This will keep SMF stuff in one file and jQuery plugins in a file of their own, which makes more sense.

It also means slightly less js loaded in total if somebody switches from hover menus to click (like if they change from phone to desktop, for example) since function smf_addButton will only ever be loaded from the one file.*

Sorted. :)

*Damn. No it won't. :D That would only apply if they changed menu preference while on the same device. Ok, other points still make sense.

Advertisement: