MyBB Community Forums

Description:
If I'll put a popup menu near the end of the page, while the popup has an ancestor element with position 'relative' / 'absolute' / 'fixed', the popup will be partially outside the page.

Why the bug occur?
The JS code try to calculate the position where the popup should appear and than position the popup by changing CSS values. The popup menu is positioned with absolute values so normally the 'left' property will be the distance from the left side of the browser window. But if the popup has an ancestor element with position 'relative' / 'absolute' / 'fixed', the 'left' property should include only the offset from that ancestor element (and therefore, receive lower value).
Later in the code, we're trying to find out if the popup menu is partially outside the page by checking the width of the menu + its 'left' property and compare that to the width of the page (the browser window).
In this comparison the real distance from the left side of the browser window should be checked and not only the offset.

How to reproduce:
I think the 'Description' + 'Why the bug occur?' is enough but if you want to see it in your eyes, you can download my Fast Menu plugin:
http://mods.mybboard.net/download/fast-menu

Suggested Fix:
After computing the offset, continue to compute the real distance from the left side of the browser, and fix the comparison.

Here is the diff result for /jscrips/popup_menu.js:
53,63d51
< offsetTopReal = offsetTop;
< offsetLeftReal = offsetLeft;
< if (element) {
< do
< {
< offsetTopReal += element.offsetTop || 0;
< offsetLeftReal += element.offsetLeft || 0;
< element = element.offsetParent;
<
< } while(element);
< }
86c74
< if(offsetLeftReal+menuWidth >= pageSize[0])
---
> if(offsetLeft+menuWidth >= pageSize[0])

The full (fixed) /jscrips/popup_menu.js file:

var PopupMenu = Class.create();

PopupMenu.prototype = {

	initialize: function(id, options)
	{
		document.currentMenu = "";

		if(!$(id))
		{
			return false;
		}
		this.id = id;
		var element = $(id);
		
		var popupMenu = element.id+"_popup";
		if(!$(popupMenu))
		{
			return false;
		}
		
		this.menu = $(popupMenu);
		this.menu.style.display = "none";
		element.onclick = this.openMenu.bindAsEventListener(this);
	},
	
	openMenu: function(e)
	{
		Event.stop(e);
		if(document.currentMenu && document.currentMenu == this.id)
		{
			this.closeMenu();
			return false;
		}
		else if(document.currentMenu != "")
		{
			this.closeMenu();
		}
		
		offsetTop = offsetLeft = 0;
		var element = $(this.id);
		do
		{
			offsetTop += element.offsetTop || 0;
			offsetLeft += element.offsetLeft || 0;
			element = element.offsetParent;
			if(element)
			{
				if(Element.getStyle(element, 'position') == 'relative' || Element.getStyle(element, 'position') == 'absolute') break;
			}
		} while(element);
		offsetTopReal = offsetTop;
		offsetLeftReal = offsetLeft;
		if (element) {
			do
			{
				offsetTopReal += element.offsetTop || 0;
				offsetLeftReal += element.offsetLeft || 0;
				element = element.offsetParent;
				
			} while(element);
		}
		element = $(this.id);
		element.blur();
		this.menu.style.position = "absolute";
		this.menu.style.zIndex = 100;
		this.menu.style.top = (offsetTop+element.offsetHeight-1)+"px";
		// Bad browser detection - yes, only choice - yes.
		if(MyBB.browser == "opera" || MyBB.browser == "safari")
		{
			this.menu.style.top = (parseInt(this.menu.style.top)-2)+"px";
		}
		this.menu.style.left = offsetLeft+"px";
		this.menu.style.visibility = 'hidden';
		this.menu.style.display = 'block';
		if(this.menu.style.width)
		{
			menuWidth = parseInt(this.menu.style.width);
		}
		else
		{
			menuWidth = this.menu.offsetWidth;
		}
		pageSize = DomLib.getPageSize();
		if(offsetLeftReal+menuWidth >= pageSize[0])
		{
			this.menu.style.left = (offsetLeft-menuWidth+element.offsetWidth)+"px";
		}
		this.menu.style.display = 'block';	
		this.menu.style.visibility = 'visible';

		document.currentMenu = element.id;
		Event.observe(document, 'click', this.closeMenu.bindAsEventListener(this));
	},
	
	closeMenu: function()
	{
		if(!document.currentMenu)
		{
			return;
		}
		var menu = document.currentMenu;
		menu = $(menu+"_popup");
		menu.style.display = "none";
		document.currentMenu = "";
		document.onclick = function() { };
	}
};

And I've also attached the fixed file.
[attachment=11816]

What else can you request in a bug report?
I'm suggesting to stick this thread as a splendid bug report Big Grin

Update:
The IE7 problem is fixed too now.

Erm, how does this problem occur in a standard MyBB installation with no modifications?

Standard MyBB installation with no plugins and no theme nor template changes should work fine, but when was the last time you've seen such a board? Big Grin

The JS has this error of positioning, it's just doesn't occur to the current uses of unchanged MyBB, but who know what we'll need in the future? and what plugins and themes creators expect from MyBB?

(A theme creator could easily put the quick edit button near the edge of the page for example)

I've done a slightly different thing to get rid of the "break" statement, but this should work - could you double-check this?

var PopupMenu = Class.create();

PopupMenu.prototype = {

	initialize: function(id, options)
	{
		document.currentMenu = "";

		if(!$(id))
		{
			return false;
		}
		this.id = id;
		var element = $(id);
		
		var popupMenu = element.id+"_popup";
		if(!$(popupMenu))
		{
			return false;
		}
		
		this.menu = $(popupMenu);
		this.menu.style.display = "none";
		element.onclick = this.openMenu.bindAsEventListener(this);
	},
	
	openMenu: function(e)
	{
		Event.stop(e);
		if(document.currentMenu && document.currentMenu == this.id)
		{
			this.closeMenu();
			return false;
		}
		else if(document.currentMenu != "")
		{
			this.closeMenu();
		}
		
		offsetTop = offsetLeft = 0;
		var element = $(this.id);
		do
		{
			offsetTop += element.offsetTop || 0;
			offsetLeft += element.offsetLeft || 0;
			element = element.offsetParent;
			if(element)
			{
				if(Element.getStyle(element, 'position') == 'relative' || Element.getStyle(element, 'position') == 'absolute')
				{
					// calculate the true top/left position relative to page borders (this is used for checking whether the popup menu will be displayed within the page)
					offsetTopReal = offsetTop;
					offsetLeftReal = offsetLeft;
					do
					{
						offsetTopReal += element.offsetTop || 0;
						offsetLeftReal += element.offsetLeft || 0;
					} while(element = element.offsetParent);
				}
			}
		} while(element);
		element = $(this.id);
		element.blur();
		this.menu.style.position = "absolute";
		this.menu.style.zIndex = 100;
		this.menu.style.top = (offsetTop+element.offsetHeight-1)+"px";
		// Bad browser detection - yes, only choice - yes.
		if(MyBB.browser == "opera" || MyBB.browser == "safari")
		{
			this.menu.style.top = (parseInt(this.menu.style.top)-2)+"px";
		}
		this.menu.style.left = offsetLeft+"px";
		this.menu.style.visibility = 'hidden';
		this.menu.style.display = 'block';
		if(this.menu.style.width)
		{
			menuWidth = parseInt(this.menu.style.width);
		}
		else
		{
			menuWidth = this.menu.offsetWidth;
		}
		pageSize = DomLib.getPageSize();
		if(offsetLeftReal+menuWidth >= pageSize[0])
		{
			this.menu.style.left = (offsetLeft-menuWidth+element.offsetWidth)+"px";
		}
		this.menu.style.display = 'block';	
		this.menu.style.visibility = 'visible';

		document.currentMenu = element.id;
		Event.observe(document, 'click', this.closeMenu.bindAsEventListener(this));
	},
	
	closeMenu: function()
	{
		if(!document.currentMenu)
		{
			return;
		}
		var menu = document.currentMenu;
		menu = $(menu+"_popup");
		menu.style.display = "none";
		document.currentMenu = "";
		document.onclick = function() { };
	}
};

Thanks!

Hi,

Your code will have manny (normal) situations that the variable offsetLeftReal isn't even defined, anyway, that isn't the real problem. Quick fix.

But why to make a loop inside a loop???
What the problem with my code?

You're right - I knew there was something missing.

Yours is fine, I just thought that if there was the chance, to remove the "break" statement. Guess it doesn't work. Thanks.

So you agree that this should be fixed?
I can't understand why not... and even after there is a suggested fix...

(2008-11-29, 09:40 PM)dvb Wrote: [ -> ]Standard MyBB installation with no plugins and no theme nor template changes should work fine, but when was the last time you've seen such a board?

The JS has this error of positioning, it's just doesn't occur to the current uses of unchanged MyBB, but who know what we'll need in the future? and what plugins and themes creators expect from MyBB?

(A theme creator could easily put the quick edit button near the edge of the page for example)

Just hang on a sec whilst I push it in.
All default MyBB popup menus work here, so I'll take your word that it works for your special case.

Thank you for your bug report.

This bug has been fixed in our internal code repository. Please note that the problem will not be fixed here until these forums are updated.

With regards,
MyBB Group

Thanks, appreciate every minute you devote to MyBB Big Grin

dvb

Ryan Gordon

dvb

Yumi

dvb

Yumi

dvb

Yumi

Yumi

dvb