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

RESOLVED FIXED in mozilla0.9.7

Status

()

Core
Layout
P2
normal
RESOLVED FIXED
16 years ago
16 years ago

People

(Reporter: /\/\arcio Galli, Assigned: Marc Attinasi)

Tracking

Trunk
mozilla0.9.7
x86
Other
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

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

Comment 1

16 years ago
Created attachment 47904 [details]
link 1 make the SPAN display="none", link 2 -> display "inline"
(Reporter)

Comment 2

16 years ago
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. 
(Reporter)

Comment 3

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

Comment 4

16 years ago
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.
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...
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 6

16 years ago
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.
*** Bug 87837 has been marked as a duplicate of this bug. ***
Over to layout to investigate; it looks like the DOM is doing the right thing....
Assignee: jst → attinasi
Component: DOM Style → Layout
QA Contact: ian → petersen
*** Bug 105130 has been marked as a duplicate of this bug. ***
*** Bug 105130 has been marked as a duplicate of this bug. ***
Summary: DOM styling fails when alternating display property of SPAN element (with child DIV element). → DOM styling fails when alternating display property of inline element with child block.

Updated

16 years ago
Keywords: top100
Target Milestone: --- → mozilla0.9.7

Comment 11

16 years ago
Dang, this smells a little like (at least part of) bug 56894...
Status: NEW → ASSIGNED

Updated

16 years ago
Priority: -- → P2
Summary: DOM styling fails when alternating display property of inline element with child block. → [IB]DOM styling fails when alternating display property of inline element with child block.

Comment 12

16 years ago
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

16 years ago
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.
Depends on: 56894

Comment 14

16 years ago
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

16 years ago
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

16 years ago
Created attachment 59433 [details] [diff] [review]
PATCH to clear style data for inlineStyle changes before a reframe

Comment 17

16 years ago
CC'ing Hyatt - he needs to review this change.
Summary: [IB]DOM styling fails when alternating display property of inline element with child block. → [FIX][IB]DOM styling fails when alternating display property of inline element with child block.

Comment 18

16 years ago
That looks right to me.  sr=hyatt

Comment 19

16 years ago
requesting review now...
Assignee: attinasi → attinasi_layout
Status: ASSIGNED → NEW

Updated

16 years ago
Status: NEW → ASSIGNED

Comment 20

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

good catch. r=pierre
Attachment #59433 - Flags: review+

Comment 21

16 years ago
Fix is in:

/cvsroot/mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp,v  <-- 
nsCSSFrameConstructor.cpp
new revision: 1.664; previous revision: 1.663
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 22

16 years ago
Any luck that your patch is also fixing bug 105619?

Comment 23

16 years ago
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!
You need to log in before you can comment on or make changes to this bug.