100302 - .display and .visibility broken in this build

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[Brad Kemper]

From Bugzilla Helper:
User-Agent: Mozilla/4.0 (compatible; MSIE 5.0; Mac_PowerPC)
BuildID:    2001091311

Menu system in example worked in 0.9.2, but not 0.9.4. Javascript no 
longer seems to recognize when an element is visible or not. Also, wrong 
elements have their visibility changed.

Reproducible: Always
Steps to Reproduce:
1. go to http://providentcu.org
2. click on "Products"
3. Note which item is displayed
4. click on "Account Manager"
5. Note which item is displayed

Actual Results:  wrong items have their visibility changed, and other 
elements do not have their display hidden.

Expected Results:  Clicking on "Products" should disply links underneath 
that graphic in the navigation bar, while hiding and collapsing the links 
under the other graphics in the navigation bar. This worked in 0.9.2.

View source for frameset and look at function expand(s), specifically the 
section that begins "else if (nn5)". That function is called from the 
navigation bar frame, which sends it the argument identifying which 
graphic was clicked on (1-6). Links have IDs such as m3Child and 
catagories have IDs such as m3Parent.

Comment 1

[Brad Kemper]

What seems to happen is that the item that gets displayed is the one you 
clicked on previously.

Comment 2

[Jerome Lacoste]

What one can note as well is that the first menu item is expended but not
displayed when the page is first rendered. At least with 2001091303 on W2k.
Bg doesn't appear on IE5.
I would also like to note that, and I don't know if it related, but once
expended the menus can't be reminimised (by clicking on them). I didn't check
for duplicate on that other one.
The site contains a lot of Javascript and it is not possible to open the left
frame only using the right click button 'Show only this frame' without disabling
Javascript.
Should move as confirmed

Comment 3

[Greg K.]

Confirmed using Mac/2001091311 (0.9.4).

Comment 4

[Johnny Stenback (:jst)]

Could someone simplify this testcase?

Comment 5

[Brad Kemper]

Attachment: Has a <P> class inside a <DIV> class

Comment 6

[Brad Kemper]

Attachment: Just Divs (no class within class), still problems.

Comment 7

[Brad Kemper]

I just attached two sample files pared down to the bare essentials (plus 
the IE code, so you can see it work correctly). In both of them, nothing 
works unless you click on the same link a second time. Try clicking on 
each of the links once, first, in random order, then click  on another that 
you clicked earlier. In providentcu2.html, it will work more or less correctly 
after you click on one of the links a second time. In providentcu.html the 
behavior is simila, except the visibility is changed for the previous element 
you clicked on instead of the current one, and the last 2 are expanded at a 
time instead of just the one you click on.

Comment 8

[Johnny Stenback (:jst)]

Brad, your "Just divs" testcase is broken, you expect style.display to be "none"
the first time one of those links are clicked, but it won't be "none", there's
nothing in the code that says an elements *inline* style should be none.

The other testcase has the same problem.

Marking future for now.

Comment 9

[Brad Kemper]

Actually, I have had a chance to look at this again, and the last comment is 
not true. The style.display IS set to none BEFORE the first link is clicked. It 
is set in the style sheets. Specifically, with the line:

write(".child  { width:200px; display:none; }");

and then the initial class of the element is "child".

Comment 10

[Brad Kemper]

I just tested in Mozilla .9.7, and the problem still exists.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Setting style via a class does not affect inline style (the "style" attribute).
 You should be using getComputedStyle to get a value of display that accounts
for style rules in various user/author/agent sheets.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Attachment: Just Divs testcase that checks for current display correctly.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Oh, and the original site works for me in build 2002-01-19-21, on linux....

Comment 14

[Brad Kemper]

Could you elaborate on comment number 11? I don't understand. Since 
when can I not set the inline style via a class? I always could before, and 
in every other browser, icluding earlier builds of mozilla.

Comment 15

[Boris Zbarsky [:bzbarsky]]

The "style" property of a node reflects the inline style, which is set via the
"style" property of the element in the HTML.  One can have style rules that
apply to the node that are in a stylesheet.  Example:

<style>
  .foo { display: none }
</style>

<div class="foo" id="one" />
<div id="two" style="display:none" />

Now, document.getElementById("two").style.display is "none", since the display
is set in the inline style.  Bug document.getElementById("two").style.display is
"", since the display is _not_ set in the inline style and since the "style"
property only reflects the inline style.  The way to find out the _actual_ style
of the node, taking into account inline style, user sheets, UA sheets, and
author sheets is via getComputedStyle (as in the example I attached) or via IE's
proprietary currentStyle object.

> I always could before, and in every other browser, icluding earlier builds of
> mozilla.

IE has some bugs in its handling of the interaction of non-inline rules and
inline style.  In _most_ cases the non-inline rules are only visible via the
currentStyle object and not vixible in the inline style, but it screws up
sometimes.  Mozilla has not reflected non-inline rules in the inline style
object since at least a year and a half ago; I can't comment on the state of
things before that because I was not working on the project then.

Does that help?  See
http://www.w3.org/TR/2000/REC-DOM-Level-2-Style-20001113/css.html#CSS-ElementCSSInlineStyle
for more information on inline style and
http://www.w3.org/TR/2000/REC-DOM-Level-2-Style-20001113/css.html#CSS-ViewCSS
for information on getComputedStyle.

Comment 16

[Prashant Desale]

Severity = LOW [No Crash, functional failure, No Cosmetic failure]
Visibility = HIGH [One point on Real world website usage, One more point of 
compatibility with other browsers, since reporter says it used to work on older 
builds & I tested it with IE & it works. gets another more point on compliance 
with adopted techonology] 

Priority = Visibility * Severity

Priority = p3

adding word "qawanted" because I'm setting this priority on available data & if 
someone feels otherwise then please investigate this more & feel free to change 
this priority.

Comment 17

[Boris Zbarsky [:bzbarsky]]

*** Bug 131072 has been marked as a duplicate of this bug. ***

Comment 18

[Boris Zbarsky [:bzbarsky]]

*** Bug 142875 has been marked as a duplicate of this bug. ***

Comment 19

[Brad Kemper]

Well, as author of the providentcu.org site, I spent the time to follow the 
suggestions for coding changes, and actually got the navigation system 
for the site to work great... in Netscape 6.1. Unfortunately, it still does not 
work correctly in Netscape 6.2 or any of the recent builds of Mozilla. 
Behavior on each release seems to be different. In release candidate 1, 
the menus do not close, and when they open they are not visible. 

The degree of effort to support this mozilla code is mind-boggling. Will I 
need to branch out the javascript based on whether the user has NN6, 
NN6.1, NN6.2, or the various versions of Mozilla? I am more likely to just 
scrap the advanced features for anyone using Mozilla/Netscape code 
greater than 6.1, if I cannot come up with something that works 
reasonably well for all.

Consider that by following the advice on this page I needed code like the 
following:

window.Navigation.document.defaultView.getElementById("m1Child").get
ComputedStyle(window.Navigation.document.defaultView.getElementByI
d("m1Child"), "").getPropertyValue("display")

instead of something as simple as this in IE:
window.Navigation.document.all["m1Child"].style.display

Now, I don't mind using "getElementById( )" instead of "all[ ]", but none of 
my books say anything about "getPropertyValue" or "defaultView" or 
"getComputedStyle", and the w3 links on this page do not help elucidate 
anything, as they seem to be written as inscutably as possible, with no 
examples. Sorry about the rant, but what is it going to take?

Comment 20

[Boris Zbarsky [:bzbarsky]]

window.Navigation.document.defaultView.getElementById("m1Child").get
ComputedStyle(window.Navigation.document.defaultView.getElementByI
d("m1Child"), "").getPropertyValue("display")

You mean

var doc = window.Navigation.document;
doc.defaultView.getComputedStyle(doc.getElementById("m1Child"),
"").getPropertyValue("display");

You have some extraneous "defaultView" and getElementById calls in what you typed...

I'm looking at the code right now to see why it works in NS6.1 and fails in
later releases... none of this stuff should have changed since 6.1

Comment 21

[Boris Zbarsky [:bzbarsky]]

And I see your point about it being much more complicated than IE, by the way.  :)

Comment 22

[Boris Zbarsky [:bzbarsky]]

You're setting the layers to "display:inline" to show them.  But they are table
cells.  The correct display value for a table cell is "table-cell" (see
http://www.w3.org/TR/REC-CSS2/visuren.html#propdef-display and
http://www.w3.org/TR/REC-CSS2/tables.html#q2).  We _do_ have a bug which
requests that we handle non-table content in tables slightly better...  And our
handling of this situation _has_ gotten a bit worse since NS6.1.

In any case, replacing "inline" with "table-cell" in the nn5 sections should
make this code work identically in all the Mozilla and Netscape versions involved...

Comment 23

[Brad Kemper]

Oh, thank you. I hope it works; I will give it a shot tomorrow or so. What is 
the bug on non-table content?

Comment 24

[Brad Kemper]

By the way, I could not find any information on why "defaultView" is 
needed, or what the second argument for "getElementById" is. Any help, 
especially with examples?

Comment 25

[Boris Zbarsky [:bzbarsky]]

The bug I was thinking of is bug 97506, which is actually about table rows, not
table cells.... if fixing that does not fix this problem, another bug should
likely be opened on it.

For why "defaultView" is necessary, according to
http://www.w3.org/TR/2000/REC-DOM-Level-2-Style-20001113/css.html#CSS-ViewCSS
you need an instance of the ViewCSS interface (which inherits from the
views::AbstractView) interface to call getComputedStyle.  Looking at the "views"
specification, we see that the only property that returns an AbstractView is the
defaultView property on the DocumentView interface
(http://www.w3.org/TR/2000/REC-DOM-Level-2-Views-20001113/views.html#Views-DocumentView).
 "document" implements the DocumentView interface, and we unwind from there...

The second argument for getComputedStyle is a pseudo-element.  So for example
you could do:

getComputedStyle(foo, ":before");

and that would get the computed style for the :before pseudo-element attached to
the "foo" object.  If you want the computed style for the "foo" object itself,
the pseudo should be null or "" (and needs to be "" in NS6.0/6.1/6.2 due to a
bug....)

Comment 26

[Brad Kemper]

This seems to work now, by following the complex coding examples 
given. Thank you for your help!