Wildbit

The Blog

Thoughts on building web apps, businesses, and virtual teams.

22 May Modalbox & Lightbox conflicts ← Go back

Posted by Andrew Okonetchnikov on May 22, 2007 — 6 Comments

Andrew Okonetchnikov

I recently got an odd bug report for the ModalBox project: Modalbox used to appear with the wrong positioning. The modal window was sliding down not in the center, but from the left or right side of the window (depending on browser used). So I started investigating the issue.

The Problem

Since ModalBox has unit-tests now I asked the person who reported the issue to run them first to be sure he’s using compatible prototype and script.aculo.us libraries. The tests passed successfully. Weird!

I looked into the HTML source where the issue appeared and noticed there was also a lightbox.js – another JavaScript library for creating neat popups. After excluding Lightbox from the HTML source ModalBox started to work perfectly. The issue was localized at last. Cool!

Digging the latest Lightbox (2.03.3) sources I found these lines of code:


Object.extend(Element, {
getWidth: function(element) {
element = $(element);
return element.offsetWidth;
},
...

To feel the difference here is the original getWidth and getDimensions methods from prototype 1.5.1:


getWidth: function(element) {
return $(element).getDimensions().width;
},
getDimensions: function(element) {
element = $(element);
var display = $(element).getStyle('display');
if (display != 'none' && display != null) // Safari bug
return {width: element.offsetWidth, height: element.offsetHeight};
// All *Width and *Height properties give 0 on elements with display none,
// so enable the element temporarily
var els = element.style;
var originalVisibility = els.visibility;
var originalPosition = els.position;
var originalDisplay = els.display;
els.visibility = 'hidden';
els.position = 'absolute';
els.display = 'block';
var originalWidth = element.clientWidth;
var originalHeight = element.clientHeight;
els.display = originalDisplay;
els.position = originalPosition;
els.visibility = originalVisibility;
return {width: originalWidth, height: originalHeight};
}

As you can see, the original method handles the case when the element is not visible at the time of calling the getWidth method. It’s very useful when you need to calculate the dimensions of the element before showing it. ModalBox relies on this technique as well.

Bad design

For sure, Object.extend is a very useful way to override existing methods with your own ones. But shouldn’t it be used with care? To me it’s a good example of bad design as well. Thinking that developers will use only one (and only your own) JavaScript component seems stupid to me these days. Overriding some basic methods is very dangerous practice. This particular example just demonstrated why.

Conclusion

Be aware of using prototype’s Object.extend method without keeping in mind the rest of the world. Mostly, people rely on how each particular framework is working. Silently overriding a framework’s core functionality isn’t a good programming practice.

6 Comments

Nice catch.

Dmitry Sabanin — May 23, 2007, 7:58 pm

Dima Sabanin

How do you resolve the problem?

Dan — January 9, 2008, 8:54 pm

God job Andrew, i got the problem too,
So what do i have to fix that?

Do i have to ask that developer to rewrite his code?

Seandy — January 22, 2008, 12:44 pm

I actually discovered this same issue and resolved it before reading this article (but the article could have saved me a lot of time). I fixed the issue by removing lines 100-103 from lightbox.js and both Lightbox and Modalbox seem to work fine. I also sent an e-mail to the author documenting the issue.

Ian — February 1, 2008, 10:56 am

Hey, i found on line 412 of modalbox.js:

Element.setStyle(this.MBwindow, {left: Math.round((Element.getWidth(document.body) – Element.getWidth(this.MBwindow)) / 2 ) + “px”});

But I and i’m sure others often give the XHTML body a CSS style of width, and “margin:0 auto;” which makes the page horizontally centered but with a body of fixed width. By changing line 412 to:

Element.setStyle(this.MBwindow, {left: Math.round((Element.getWidth(document.viewport) – Element.getWidth(this.MBwindow)) / 2 ) + “px”});

I got it to center correctly.

Jeffrey Warren — July 1, 2008, 8:23 pm

1) I’m found this. if CSS exists this text:

body
{
margin: 6px 0 10px;
padding: 4px;
}

IE7 bug

2)

show my form

settings:

test value default
test value
test value selected

If I’m click “show my form”, IE7 show me “test value default”, but not “test value selected”

Opera and Mozilla is good (“test value selected”)

Mr_ZLO — May 20, 2009, 12:55 am

Write a comment

* required
* required

← Go back

Get in Touch

Wildbit, LLC

Work 20 North 3rd St, 701
Philadelphia, PA 19106 USA

Google Maps
 
 
Fax
+1 (267) 200 0835
Email
IndyHall

We work at IndyHall. Coworking is more than just space.