Simple Machines Community Forum

Customizing SMF => SMF Coding Discussion => Topic started by: Antechinus on January 15, 2015, 11:12:20 PM

Title: Odd (minor) javascript error
Post by: Antechinus on January 15, 2015, 11:12:20 PM
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?
Title: Re: Odd (minor) javascript error
Post by: Arantor on January 15, 2015, 11:19:06 PM
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)
Title: Re: Odd (minor) javascript error
Post by: Antechinus on January 15, 2015, 11:22:57 PM
Aha. Got it. So what does the code do though, since my images are expanding and contracting on click anyway?
Title: Re: Odd (minor) javascript error
Post by: Arantor on January 15, 2015, 11:24:45 PM
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.
Title: Re: Odd (minor) javascript error
Post by: Antechinus on January 15, 2015, 11:26:54 PM
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.
Title: Re: Odd (minor) javascript error
Post by: Arantor on January 15, 2015, 11:29:18 PM
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.
Title: Re: Odd (minor) javascript error
Post by: Antechinus on January 15, 2015, 11:40:14 PM
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.
Title: Re: Odd (minor) javascript error
Post by: Arantor on January 15, 2015, 11:45:32 PM
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.
Title: Re: Odd (minor) javascript error
Post by: Antechinus on January 15, 2015, 11:49:28 PM
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)}
Title: Re: Odd (minor) javascript error
Post by: Antechinus on January 15, 2015, 11:54:54 PM
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.
Title: Re: Odd (minor) javascript error
Post by: Arantor on January 15, 2015, 11:55:58 PM
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...
Title: Re: Odd (minor) javascript error
Post by: Antechinus on January 16, 2015, 12:04:38 AM
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.
Title: Re: Odd (minor) javascript error
Post by: Antechinus on January 16, 2015, 12:07:54 AM
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.
Title: Re: Odd (minor) javascript error
Post by: Arantor on January 16, 2015, 12:14:08 AM
Ahhhh, yes.
Title: Re: Odd (minor) javascript error
Post by: Antechinus on January 16, 2015, 12:18:07 AM
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
Title: Re: Odd (minor) javascript error
Post by: Arantor on January 16, 2015, 12:23:35 AM
Yup, that's it. Nicely figured out :)
Title: Re: Odd (minor) javascript error
Post by: Antechinus on January 16, 2015, 12:42:06 AM
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.