Friday, January 30, 2009

Fixing Liferay 5.1.2 in Internet Explorer 6

Here's a fix from Nate that doesn't remove the prerendering: LPS-4260
(This fix is in the liferay subversion trunk now so you can just get the latest or look at the fairly small diffs and merge just the parts that he did with your source. If you already implemented the Eric Vigna fix, you should remove that first before using Nate's changes)

Ignore absolutely everything I wrote and just look here for a fix.
(The compromise is that pre-rendering the manage pages list is disabled. Thanks, Eric Vigna)

In Liferay 5.1.2 IE6 users (everybody at my company for some stupid reason) can't Manage Pages in Liferay because their way of using of the treeview plugin breaks. I have my own treeview stuff that works fine in IE6, but something they did breaks it in a way that is completely impossible to debug using the resources available for IE6.

I made this work by putting the following in my custom theme's document.ready function inside javascript.js:
/* Fix liferay's broken tree control for IE6 on the Manage Pages portlet */
if ( $.browser.msie && /6.0/.test(navigator.userAgent) ) {
$(".lfr-tree").find("ul").addClass("node-open")
.css("display","list-item")
.css("position","fixed")
.css("background-image","none");
$(".lfr-tree").find("li").css("display","list-item");
}

It's ugly code and a little slow, but I couldn't care less. I think one "right" way to write this would be to change portal-web/docroot/html/js/liferay/tree.js to not hide the stuff in the first place when it creates the menu.


Side notes: this method of browser detection is deprecated in jQuery 1.3 with no apparent replacement and the Liferay forum is still either broken or locked.

Update: I swear I tested this on Friday and achieved failure, but apparently just doing the CSS stuff makes the treeview clickable.
So just put this inside custom.css:
.ie6 ul.node-open{
display:inline;
position: static;
background-image:none;
}
.ie6 ul.node-open li{
background-image: none;
}
I tested commenting out each line of this to make sure nothing was there that is unnecessary and found this to be the minimum to make it actually work. Also, Nate was right, but setting position:fixed was defaulting the ul position to be static, so it still worked since we just needed to make it not absolute. The background-image stuff isn't strictly necessary, but you get some blue arrows that are overlapped by your list controls if you don't include it.

3 comments:

Nate Cavanaugh said...

Hi Matt,
I'm one of the UI guys over at Liferay and I was a little curious as to what you meant by "completely unusable".

Also, I thought I'd let you know that Liferay has had it's own browser detection for quite a while, and in 5.1.x, the detection methods are unified.

So to detect a browser, you would do, for example:

if(Liferay.Browser.isIe() && Liferay.Browser.getMajorVersion() <= 6){
..custom code...
}

It also supports, AIR, the iPhone, solaris, and a bunch more that jQuery doesn't support.

But a couple of things: One is that it looks like you're doing primarily CSS changes.
You could theoretically just the do the CSS in the theme, using the custom browser selectors, such as .ie6, etc.
Two, the line:
.css('position', 'fixed')
doesn't do anything in IE6 because position: fixed wasn't supported until IE7.

Lastly, the reason we don't open them by default is that there are customers who literally have upwards of 3000+ pages, and rendering them all out can be a bit overwhelming.
And since choosing "Expand All" or "Collapse All"
will persist that setting, it ends up not being a huge deal to most users.

But if you could describe the issue here (or better yet, at http://issues.liferay.com and file a bug report and assign it to me), I would be able to look at the underlying issue.

Thanks Matt,

he said said...

Hi Nate!
I don't know what I called completely unusable. If you mean the forums, then yeah, I've been trying to post this and the last blog post for three days now. If you mean debugging the javascript, that's more representative of the fact that I was debugging using the Microsoft Script Debugger (since the problem only occurs in IE6, there's not much choice besides visual studio or MS Script Debugger).

If you mean the basic bug description, it can be summed up as "clicking the little arrows doesn't do anything except change the arrow icon when using IE6". Unfortunately our company policy is no IE7 still, so this was a big deal for us.

Thanks for the browser detection tip. I just copied that line out of the bgiframe plugin. As to the position:fixed bit, it didn't work without it. I don't know why. Relative didn't work, and web developer toolbar showed absolute to be the default. Lastly, I did it in the javascript because it's Friday and I wanted to go home and that saved me precious seconds of development over deciding which css file it actually belonged in.

Also, I meant open by default for IE6 only. Bad phrasing on my part and I seriously hope your big customers are enlightened enough to be off IE6.

Thanks for the quick response! I'll edit the post later (Monday, maybe) to include what you said.

he said said...

Also, it was already filed as LEP-6255. Maybe also as LPS-643? I'll put a comment on LEP-6255 later too.