97874 - [FIX][IB]DOM styling fails when alternating display property of inline element with child block.

Status: RESOLVED FIXED

(Core :: Layout, defect, P2)

Description

[/\/\arcio Galli]

Please ignore the URL. :-) is the original web site I found the problem. 
I will attach the testcase then the description.

Comment 1

[/\/\arcio Galli]

Attachment: link 1 make the SPAN display="none", link 2 -> display "inline"

Comment 2

[/\/\arcio Galli]

The test case illustrates the problem. Basically there is a 

<SPAN><DIV>blingblong</DIV></SPAN>

click test1 and will apply display="none" to the CONTENT blingblong.
click test2 and will apply display="inline" to the CONTENT blingblong.
click test1 again and display="none" does not work anymore. 

once apply inline.. then none does not work. You will also be able to see the
effect in the URL, look the news area in the top. 

Comment 3

[/\/\arcio Galli]

Tested with 
Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.3+) Gecko/20010831

Comment 4

[Fabian Guisset]

Hey Marcio :-)
A div cannot be the child of a span, because a span is display:inline and a div
is display:block. I'm not sure why you would want to put a div inside a span.
-Fabian.

Comment 5

[Boris Zbarsky [:bzbarsky]]

setting status to new.  I can understand the div not disappearing, but if I do
something like:

<span>aaa <div>text</div> </span>

Then the "aaa" does not disappear either...  This looks like something goes
interestingly wrong with the frames in this case...

Comment 6

[/\/\arcio Galli]

Thanks guys for the diagnose. Yes, the point is not really about the use of span
with div as child,.. but once (if) we apply display=inline (test2) to the span,
the first testcase does not work anymore display=none.

The original page used the span just as a container to select display of the
news area. Anyway, I already have one evangelism incident trying to have this
web site fixed.

Comment 7

[Boris Zbarsky [:bzbarsky]]

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

Comment 8

[Boris Zbarsky [:bzbarsky]]

Over to layout to investigate; it looks like the DOM is doing the right thing....

Comment 9

[Boris Zbarsky [:bzbarsky]]

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

Comment 10

[Boris Zbarsky [:bzbarsky]]

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

Comment 11

[Marc Attinasi]

Dang, this smells a little like (at least part of) bug 56894...

Comment 12

[Marc Attinasi]

I cannot see this problem on msnbc.com, and the original report says to ignore
that URL, so I am clearing it. 

If there are any real sites showing this problem, please put that in this bug,
it will increase the priority. Basically, I know this is a problem but I cannot
give it much priority without a real site at this time.

Comment 13

[George Dittmeier]

Try looking at 
http://www.datstat.com/mozillabug-2.html 
-and on a related note...watch what happens to input fields!-
http://www.datstat.com/mozilla-bug-3a.html

This should really be considered a major bug.

Comment 14

[Marc Attinasi]

The problem is this: if an inline element that has a block child has its display
property set via the style attribute (either in the markup of by the DOM), or it
has an out-of-line style rule with sufficient weight (!important) then it cannot
be changed to display:none.

Since the DOM changes the style attribute of an element, the testcase attached
shows the case where style="display:inline" cannot be changed to 'display:none'.
The same result can be seen without using the DOM to set the display:inline by
using markup like this:

<span id="test1" style="display:inline;">
  <div>NEWS 1 NEWS 1 NEWS 1</div>
</span>
<hr>
<a href=""
onclick="javascript:document.getElementById('test1').style.display='none';return
false">display:none</a>
</body></html>
The root of the problem in the FrameConstructor is that the style context for
the element that has had its display set to 'none' is showing a display of
'inline'. Since the display is 'inline' and not 'none', it does not disappear.
Interestingly, if I dump the content I see that the style attribute has been
changed to style="display:none' - it appears that it is just the style context
that has not been updated correctly.

Comment 15

[Marc Attinasi]

Actually, we are Resolving style again, but the style system is not resolving
the display:none correctly. The problem is, we are not clearing the style data
from the style set like we should when there is an inlineStyle change. Patch
coming up...

Comment 16

[Marc Attinasi]

Attachment: PATCH to clear style data for inlineStyle changes before a reframe

Comment 17

[Marc Attinasi]

CC'ing Hyatt - he needs to review this change.

Comment 18

[David Hyatt]

That looks right to me.  sr=hyatt

Comment 19

[Marc Attinasi]

requesting review now...

Comment 20

[Pierre Saslawsky]

Comment on attachment 59433 [details] [diff] [review]
PATCH to clear style data for inlineStyle changes before a reframe

good catch. r=pierre

Comment 21

[Marc Attinasi]

Fix is in:

/cvsroot/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp,v  <-- 
nsCSSFrameConstructor.cpp
new revision: 1.664; previous revision: 1.663

Comment 22

[rbs]

Any luck that your patch is also fixing bug 105619?

Comment 23

[Marc Attinasi]

rbs - the testcase and URLs in bug 105619 do not crash in my build. I'll make a
comment in that bug so somebody can check it out (I have other changes in my
tree as well, so I cannot be sure it was this patch). Good catch!