#3595 closed defect (bug) (fixed)
Menu still not working with 1.5 and IE7
Reported by: | blogsterfiles | Owned by: | |
---|---|---|---|
Milestone: | 1.6 | Priority: | normal |
Severity: | major | Version: | 1.5 |
Component: | Templates | Keywords: | needs-patch |
Cc: |
Description
In default mode, BP 1.5 menus are still malformed. If you click the blue compatibility button (see img), it seems to correct the problem when the button turns from blue to gray, but it's like pulling hen's teeth to get site visitors to take that simple step with their browser. Image is from testbp.org, but same thing is still happening on 1.5 release just uploaded.
Attachments (6)
Change History (55)
#1
@
13 years ago
- Component changed from Core to Theme
- Keywords needs-patch added
- Milestone changed from Awaiting Review to 1.5.1
- Version set to 1.5
#2
@
13 years ago
- Severity changed from normal to major
Just wondering if anyone has any ideas on this. 1.5 menus still do not work with IE unless in compatibility mode! Am I the only one who thinks this is a major issue (not being facetious, just wondering if I'm missing something)? Anyway, BP 1.5 is awesome and I blame Microsoft for having a crappy browser, but still, BP is useless to me as long as 100% of visitors using IE see a broken site when they visit. Much appreciated if anyone has any suggestions or even a temp fix or some idea if it will be addressed in the future or not. Thanks!
#3
@
13 years ago
I think it's important, but it will not get fixed unless someone who understands IE9's rendering is able to provide a patch. Do you have any suggestions?
#4
@
13 years ago
I'm seeing menus appear the same way with IE7. Hopefully, I can investigate this weekend.
#7
follow-up:
↓ 8
@
13 years ago
- Keywords has-patch needs-testing added; needs-patch removed
This patch addresses the issue for IE7; needs testing in IE9.
When I have a chance, I'll test on IE9 (on my other com that doesn't have my debug tools) unless someone beats me to it!
---
EDIT: You can change the height for #navigation to 63px and that will address #3588 as well.
#8
in reply to:
↑ 7
@
13 years ago
I'll leave it to someone more quailfied than me to determine if this is resolved or not, but a quick test with IE9 seems to work fine with r-a-y's patch! Thanks very very much to r-a-y and everyone who responded! What a great community...awesome work!
I noticed radius corners are still broken with IE9 in default mode, but work now in compatability mode. I'm off to see if someone has a ticket going on that now...thanks again everyone!
#9
@
13 years ago
May have spoke too soon. When I added drop down menus and went to the "Home" tab, focus was on both "Home" and "Blogs" tabs. Pointing to the tabs with drop down menus, they appear and work perfectly, and as soon as I click on any other tab, everything works fine, including focus leaving the "Home" tab and being only on whichever top level tab is clicked. (logged in or logged out makes no differance)
Update: Focus is not an issue; upon deleting and recreating the menu, the problem vanished.
#10
@
13 years ago
I don't believe this is the fault of the patch, but rather a BuddyPress theme issue.
If you can test this on another browser like Firefox or Chrome and can duplicate the issue, then please open another ticket about this.
#11
@
13 years ago
- Keywords needs-testing removed
- Summary changed from Menu still not working with 1.5 and IE9 to Menu still not working with 1.5 and IE7
This bug only exists in IE7. The OP has IE9 set up to run any page in compatibility mode, which defaults to IE7 mode.
#12
@
13 years ago
r-a-y (or anyone else with IE7 knowledge) - any leads on a fix? This is the one ticket left in the milestone that I would *really* prefer not to punt.
#13
@
13 years ago
01.patch works from the testing I've done.
Try the patch on all other browsers as well. I've tested in Firefox, Chrome, Safari and Opera and haven't found any conflicts (thus far). Even works in IE6 (except for the dropdown menu since IE6 doesn't support the :hover CSS selector, but that's just IE6 being IE6!).
The problem blogsterfiles notes above could be an issue with WP menus and I believe is separate.
The only minor issue that could arise is how this affects child themes; BP TPack users won't be affected at all.
The declaration that I think would affect child themes is:
#navigation {height:70px;}
I needed to do this so the height of the #header stays intact since I'm using position:absolute
in #nav
.
Of course, feel free to tweak the CSS!
#14
@
13 years ago
Oh, cool. I have tested 01.patch on a slew of browsers and it all looks good to me.
Not sure what to say about the #navigation height issue. I'm sure it will affect at least some child themes. The change means that the vertical space between the tabs and the search box is dictated by the height of the #navigation element rather than the top margin on #nav (you can change 40px to whatever you want since it's now absolutely positioned). That seems like a pretty significant change the way that the layout is determined. And the switch to absolute positioning seems like a change for the worse.
So. A couple questions.
- How committed are we to IE7 support? Enough to make a significant CSS change mid-cycle, just to support it?
- Are there other approaches? Seems like the root of the problem is display: inline-block on #nav li, which IE7 doesn't know what to do with. It's possible that we could work around this with some browser hacks in our existing stylesheet (see eg http://grasshopperpebbles.com/css/css-inline-block-ie7-hack/), or with a conditionally loaded < IE7 stylesheet. Both of these options are icky in themselves, of course, but is it worth considering the trade-offs?
- Am I overreacting to r-a-y's proposed change? I'm not much of a front-end guy, maybe I'm making a mountain out of a molehill and we should just go with his proposal :)
#15
@
13 years ago
I'm a bit concerned about setting the height on #navigation w.r.t #3588 - the impact on themes that have already set custom header images.
#16
@
13 years ago
Seems like the root of the problem is display: inline-block on #nav li, which IE7 doesn't know what to do with. It's possible that we could work around this with some browser hacks in our existing stylesheet (see eg http://grasshopperpebbles.com/css/css-inline-block-ie7-hack/), or with a conditionally loaded < IE7 stylesheet. Both of these options are icky in themselves, of course, but is it worth considering the trade-offs?
That's correct. Though the browser hacks create problems for the tabs and dropdown menus when you hover hover them.
---
My approach uses absolute positioning because I'm using float
in #nav li
. A potential workaround in my patch instead of setting the height is adding a clear
div before closing the #navigation
div.
EDIT: Spoke too soon! Let me experiment a bit more.
#17
@
13 years ago
02.patch is a little cleaner, but introduces an empty div to clear the float.
No absolute positioning here! Cons are the empty div and child themes that have overridden header.php
#18
@
13 years ago
03.patch uses the same approach as 02, but uses the CSS :after selector to clear #navigation
. Pure CSS!
#19
@
13 years ago
3595.03.patch looks nice in all browsers I've checked, and it seems like the most elegant solution presented so far. The change in the styling here is, IMO, less intrusive than in 01.patch, as it doesn't change the height of any elements.
Feedback from another dev, who knows their browsers better than I do?
#21
@
13 years ago
The good browsers will use :after.
The workaround for IE7 is the usage of float: left
in #nav li
. Due to IE7's shoddy rendering, this makes it work without having to clear anything.
#23
@
13 years ago
karmatosed suggests trying https://github.com/scottjehl/Respond
#24
@
13 years ago
Just delved a little into this and seems 2 issues to me:
- We don't use a respond.js but have media queries- really should be as not all use full width experience of a browser.
However.. this is not the main reason :
- Totally agree with Ray it's because we don't have a float left or right (or ie 7 friendly equivalent) for the menu items we use other methods from what I can see. I fudged a fix by adding float right to the li - probably not a full fix but it shows issue.
#25
@
13 years ago
- Keywords dev-feedback added
Your patch .03 provides best solution, if inline-block was required then the correct approach would be to add a CC sheet for IE and setting display: inline - not hugely easy with WP - given the issue changing to 'float' is the best solution.
- 'display: inline-block;' is largely redundant on the li elements now, however it is a trigger for the MS construct hasLayout forcing it to '= true' which will then act to contain floats so may be left in place at no particular cost.
- For safety font-size: 0; line-height:0; ought to be added to that pseudo element to ensure no browsers try and preserve line-height/height for the element. There is an updated approach to the time honoured 'clearfix' approach which uses the display:table property to effect clearing and is somewhat lighter in properties required.
- While true that IE7 =< 7 does not grok pseudo element :after float containment is provided for on the parent #navigation element due to it having a stated dimension 'width' which is a 'hasLayout = true' trigger.
I would add this patch and close ticket.
#26
@
13 years ago
"I would add this patch and close ticket."
I agree. The patch seems to work just fine. So how/who does that? This seems like such a serious issue I'm surprised it hasn't been added already. Would love to see it in the core so I don't have to keep adding it, but in any event, thanks very much for the patch.
#28
@
13 years ago
One in five are still using IE...is there anything I can do to help get this patch added? It's fairly important I think. Thanks :)
#29
follow-up:
↓ 30
@
13 years ago
I believe 03.patch is good to go.
hnla outlined some other declarations that could be added as well:
https://buddypress.trac.wordpress.org/ticket/3595#comment:25 (point 2)
#30
in reply to:
↑ 29
@
13 years ago
Replying to r-a-y:
I believe 03.patch is good to go.
hnla outlined some other declarations that could be added as well:
https://buddypress.trac.wordpress.org/ticket/3595#comment:25 (point 2)
Indeed, 03.patch fixed the problems I was having in IE9's IE7 compatibility mode.
Thanks for the patch, r-a-y.
#32
@
13 years ago
- Keywords has-patch removed
I've applied .03.patch to trunk.
This is an important enough fix that it seems worth putting into the 1.5 branch, but in the case of a minor release, I'm far more concerned about breaking existing child themes. r-a-y or others, do you have an opinion about how safe this particular fix is for existing themes? (Looks to me like there will only be problems if the child theme makes major changes to the positioning of the parent element, which will result in odd behavior for the floated #nav, but I'm not sure how likely that is.)
#34
@
13 years ago
I haven't tested any child themes. The one declaration that might be troublesome is:
#nav {float:right;}
Should probably test against some popular child themes like Frisco and, probably, the upcoming "Status" theme.
DJPaul is correct that a change probably needs to be made to the RTL stylesheet as well.
---
If this patch indeed becomes troublesome, we can revert the patch and perhaps I'll release this as a minor plugin. I would rather existing child themes work as they're supposed to than supporting IE7 (which is losing browser market share every month).
#35
@
13 years ago
Floats without widths, historically, can cause issues, user agents have to know how to correctly shrinkwrap non dimensioned floats, and guess which never could do that :}
I'll try and check the patch against 'Status' and see how it stands up.
#36
@
12 years ago
- Keywords needs-patch added; dev-feedback removed
We need an IE7 RTL fix for this. Attaching a patch that comes close, but seems to do something wrong with the width of the page.
#37
@
12 years ago
Ouch just had a look at the patch on IE7 and there appears a lot wrong and not easy to figure where the collapse is and spent a little time with it.
Not sure if this is going to be fixable without spending a fair bit of time on it.
#38
@
12 years ago
Guys, do you really want to spend so much time trying to create a fix for IE7, which even Microsoft doesn't support anymore, and whose usage numbers are dwindling (5.4% at last count http://www.w3counter.com/trends )? IMHO, it is great that the issue has been fixed for IE9
#39
@
12 years ago
I agree with sooskriszta . IE7 is old, lets not spend to much time trying to fix something thats been broken for what seems like a long time, and is quickly becoming less and less of a issue.
#40
@
12 years ago
I agree with everyone who says we should drop support for IE7 entirely.
However, we still have to deal with IE8/9/10 and users who unknowingly use IE's Compatibility View, which uses IE7's rendering engine and is the main reason that the OP started this ticket.
The easiest solution is to add the following meta tag into the bp-default theme:
<meta http-equiv="X-UA-Compatible" content="IE=edge" >
This will tell IE to use the latest engine available on IE and will hence resolve issues on IE8+. If everyone likes this idea, I'll work up a patch. We'll have to be considerate to see if a child theme already uses this meta tag.
#41
@
12 years ago
I would have to agree with r-a-y I think, I'm not really a fan of adding these IE meta tags and hiss everytime I see the used for no good reason on sites where you know the dev hasn't really understood their purpose.
BP has not historically paid much more than lip service to IE so it is a bit late to be taking too much time out on this, I had a look and was planning to spend a little further time with it but think we ought to go with r-a-y's suggestions especially as it means we tell IE to correctly use the best rendering engine available for given version in other words NOT forcing => IE8 to use a sub standard engine.
However if we really want to pull IE7 into shape we can, I can set aside a bit of time to sort some of the issues.
#42
@
12 years ago
It would be nice to solve this with a meta tag rather than more css hacks. r-a-y, do you have any ideas how we could check for the existence of said tag without resorting to a dirty ob_start() trick?
#43
@
12 years ago
In PHP, I don't think there's a way around using object buffering to check for the meta tag.
It might be possible to do this with jQuery, but is this a good idea?
It might be better to just force the <meta> tag and let developers opt-out of this with remove_action
. This is akin to how WordPress forces a ton of <meta> bloat (like RSS feeds, link relationships, Windows Live Writer support, etc.), but allows devs to unhook them.
Chances are slim that there are bp-default child themes using this tag anyway.
#44
@
12 years ago
7 months is awful long time to have several people working on this. Let's make some decisions.
- No solving this with PHP or jQuery
- No IE7 specific hacks
- No browser specific meta tags in the theme, because we'll have to support it forever
- Maintain backwards compatibility as much as possible
If there's absolutely no way to do all of these things, it's okay to leave it as it is. If it's more broken than we left it in 1.5, let's revert and deal with it later, or announce on the dev blog we're breaking backpat for some CSS.
#45
@
12 years ago
Check out "The Default Theme" for a little more insight into what's in the pipeline. I agree this individual CSS quick is a pain, and I love that we all care so much about it. Let's take this energy and direct it towards a more long term solution. :)
#46
@
12 years ago
Lets draw a line under this firmly then based on the above; if we adhere to the list of 'don'ts' above we have a pretty hard time of it.
This is a theme issue and as such it is an issue that can be managed by devs or users if absolutely necessary.
I've also realised that the original ticket premise was/is flawed it states:
"IE9 in default mode (not compatibility mode)"
But shows a view of IE IN compatibility mode.
That there are issues in older IE browsers we have always been aware of, I'm pretty sure there was a general consensus not to spend too much time with attempting to get bp-default working with 6/7 in the past - if we were to spend the time then the only real accepted practise is to filter rules via MC CC styles.
bp-default has served well but doesn't warrant the time involved, especially given your post JJ on the next gen of bp skeletal theme files.
#47
@
12 years ago
First, it seems to me the best solution to this problem was derailed over concerns about IE7. Is that really a valid concern when IE7 represents a mere 5% and diminishing segment of browser use? Plus consider that most users who would be confused by the compatibility view button are likely to be using more recent versions of IE anyway, perhaps leveraging the merit of focusing on newer versions of IE to the point concerns over IE7 effectively are moot. I think simple triage logic would compel reasonable minds to look forward and not strain against forward progress by clinging to an arbitrary point of backwards compatibility.
Second, the genius of the current state of BuddyPress is that with the current BP-Default theme, BP is a practical out of the box solution for thousands of businesses, organizations, and individuals who are not able themselves, or can't justify the cost of hiring developers to customize a theme. While the original ticket image illustrated "IE IN compatibility mode" as hnla indicated above, I suggest that is a moot point. The fact remains that unless you manually install R-A-Y's patch, all visitors to a BP site using the BP-Default theme on IE will see the compatibility button in IE browsers, and if they click it (and somehow it seems they always do), they will see a broken site until they click it again, or use another browser, or click their tongue and leave never to return again. It is what it is.
While this debate has gone on, I've been using the patch without a single complaint, so is there any real reason to disagree with R-A-Y's comment 2 months ago that the patch is good to go? BP is awesome... ...please just put this patch in the theme so we don't have to reload it with every update and we can spend our time contemplating and enjoying the wonders to come! Life is good, you guys are great! Thanks for listening. :)
IE9 in default mode (not compatibility mode)