522771 - Showfor broken on IE7

Status: VERIFIED FIXED

(support.mozilla.org :: Knowledge Base Software, task, P1)

Description

[Matthew Middleton (:zzxc)]

A user in Live Chat is reporting that showfor content for all operating systems is appearing on IE7, rather than hiding content for other systems.  Searching through chat logs, there seem to be a number of other cases where Windows users are confused about Mac instructions in the middle of articles.

SUMO should be fully functional in all versions of IE >=7, and basic functionality should work in IE6.  Could we test to make sure this wasn't regressed?

Comment 1

[James Socol [:jsocol, :james]]

In IE 8, and IE 8's IE 7 mode, it works for me.

Stephen or Vishal, can you verify this in a real IE 7 installation?

Comment 2

[Stephen Donner [:stephend] Not actively reading bugmail]

This works for me in a "native" IE 7 build (that is, IE 7 in Windows XP in a VM).

Comment 3

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Screenshot of default IE 7 view on the test SHOWFOR article

Comment 4

[Stephen Donner [:stephend] Not actively reading bugmail]

I can confirm this, actually, now that I know what I'm looking for (thx, zzxc).

I'll upload a screenshot that shows the problem (look for the |Option| key reference, and others (that are Mac-specific).

Comment 5

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Screenshot showing the mixed-platform content (reference to the "Option" key for Windows)

Comment 6

[James Socol [:jsocol, :james]]

Just to clarify (and remind myself) this does occur in IE 8 when in IE 7 mode, now that I know exactly what to look for.

Does SHOWFOR have special markup for lists?

Comment 7

[Stephen Donner [:stephend] Not actively reading bugmail]

Testcase for this specific bug up at https://litmus.mozilla.org/show_test.cgi?id=9587 (we can change the testcase to test IE 6 and IE 8 too).

Comment 8

[Paul Craciunoiu [:paulc]]

Attachment: v1

Uses jQuery instead of previous implementation.

For what it's worth, the problem was in IE7's handling of getAttribute('className') (previously used) -- http://thicksliced.blogspot.com/2006/12/elementgetattributeclass.html (see IE7). I tried figuring this out with raw javascript but the element.className attribute wasn't properly updated in IE7.

This patch was tested on Fx3.5.3, IE 6,7,8.

Comment 9

[James Socol [:jsocol, :james]]

Comment on attachment 408138 [details] [diff] [review]
v1

It hid all options, instead of none of them. (Will include screenshot.)

Comment 10

[James Socol [:jsocol, :james]]

Attachment: screenshot

(Technically, this is IE 8 in IE 7 mode, but it matches what I saw in IE 7. I just couldn't easily get a screenshot from the IE 7 VM [no print screen key]).)

Comment 11

[James Socol [:jsocol, :james]]

Attachment: v2

setAttribute('style','...'); does not work in IE < 8. There were also some issues with using String.match()--I replaced strings with RegExps--and there was a variable being redeclared incessantly. I also reversed the order of the for loop to prevent repeated calls to NodeList.length.

Tested in:
WinXP:
- IE 6
- IE 7
- IE 8
- Firefox 3.5
- Chrome
MacOS
- Firefox 3.5
- Safari

Comment 12

[Paul Craciunoiu [:paulc]]

Comment on attachment 409141 [details] [diff] [review]
v2

Looks good on IE6,7,8, Fx3.5.3, and Chrome 3.

Comment 13

[Paul Craciunoiu [:paulc]]

I should add all of the above for WinXP. I also tested Linux Fx3.5.3.

Comment 14

[James Socol [:jsocol, :james]]

r54589 (trunk)

Comment 15

[Vishal Kamdar [:vish_moz]]

Verified, FIXED.
Verified on:
Win XP:
- IE 6
- IE 7
- Fx 3.5
Win Vista:
- IE 8
Linux:
- Fx 3.0

Comment 16

[James Socol [:jsocol, :james]]

r54600 (prod)