Closed
Bug 217604
Opened 21 years ago
Closed 21 years ago
using alternative CSS mess the z-order of elements
Categories
(Core :: Web Painting, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: gabriel.barros, Assigned: roc)
References
()
Details
(Keywords: testcase)
Attachments
(2 files)
495 bytes,
text/html
|
Details | |
24.14 KB,
patch
|
dbaron
:
review+
dbaron
:
superreview+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5a) Gecko/20030718 Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.5a) Gecko/20030718 this page has two elements that overlaps. One being the girl drawing and the other being the text "sakura no hana". both with absolut position given by the style sheet. When the page loads, with the default style sheet (no matter wich i use) the z-order of the elements are correct: the text below, and the girl above. But as soon as i chage to any of the alternate style sheets, that order gets twisted. The text go over the girl. Already tried changing the default CSS file and using the actual default as an alternate, and the problem continues, so it eliminates a bad craft css problem. I haven't forced z-order on the element's rules to be able to use the site as an example to that bug. Reproducible: Always Steps to Reproduce: 1. open a page with overlaping elements (ex: http://sakura.nazsco.com.br/). 2. change to an alternate CSS that keeps those elements overlaping. Actual Results: the z-order get twisted. Expected Results: keept the document z-order, as it does when rendering the page for the 1st time. i checked that bug with: mozilla 1.4 linux mozilla 1.5a linux mozilla 1.5a win32 firebird 0.6.1 2003-08-26 win32 also checked on Opera7 linux and it renders the css change just fine.
Reporter | ||
Comment 1•21 years ago
|
||
forgot to mention that it also happens when going back to the default style. 1. open the page. The z-order is OK. 2. chage to alternate style sheet. The z-order gets inverted. 3. chage back to the 1st style. The z-order stays inverted.
Comment 2•21 years ago
|
||
bug shows up with this testcase with linux trunk 20030827 (1.5b) at first, the red <div> is on top of the blue one switch style sheets -> blue <div> is on top. reload -> red <div> is back on top.
Comment 3•21 years ago
|
||
==> stylesystem
Assignee: dom_bugs → dbaron
Status: UNCONFIRMED → NEW
Component: DOM Style → Style System
Ever confirmed: true
Keywords: testcase
Comment 4•21 years ago
|
||
roc, this is all you, methinks.
Assignee: dbaron → roc+moz
Component: Style System → Layout: View Rendering
Assignee | ||
Comment 5•21 years ago
|
||
All me? Surely not! :-) What's happening here is that the style switch (I didn't even know you could have titled style blocks, that's nifty) is causing the frame to be recreated for the blue DIV. The new frame is placed at the end of the absolute child list, so it displays on top. I guess we need to whack nsCSSFrameConstructor (HAHA) so that frame recreation puts the new frame into the right place in its child lists.
Assignee | ||
Comment 6•21 years ago
|
||
Ah yes, check out this comment in nsCSSFrameConstructor: // If there are new absolutely positioned child frames, then notify // the parent // XXX We can't just assume these frames are being appended, we need to // determine where in the list they should be inserted... if (state.mAbsoluteItems.childList) { state.mAbsoluteItems.containingBlock->AppendFrames(aPresContext, *shell, nsLayoutAtoms::absoluteList, state.mAbsoluteItems.childList);
Assignee | ||
Comment 7•21 years ago
|
||
Finding the correct out-of-flow prev-sibling to insert after in each of the (absolute, fixed, float) cases is going to be ugly ... obviously the immediate in-flow prevSibling might not have an out-of-flow frame to insert after. I guess we can do something like this: find the last frame currently on the out-of-flow child list for which the placeholder frame is <= prevSibling in frame tree preorder. Writing the tree order test could be slightly tricky; do we have any code that already does it? I think not.
Comment 8•21 years ago
|
||
Shouldn't it actually be "find the last frame currently on the out-of-flow child list for which the content node is <= the content node of the prevSibling in the DOM tree"? And then we just use CompareTreePosition? And my apologies for assuming it was a views issue... we should really add NS_WARNINGs to places where we knowingly do the wrong thing. :(
Assignee | ||
Comment 9•21 years ago
|
||
It's easier to use the frame tree w/placeholders, I think --- that needs to reflect the content tree order ... whenever it doesn't, that's another bug. Anyway, I've implemented the frame ordering fix and it looks OK. But it alone doesn't fix this bug. Not only do we have to get the frames in the right order, we also have to get their views in the right order. Right now nsContainerFrame::CreateViewForFrame just appends the frame's view as the last child of the parent view. Actually we need the view order to reflect the frame tree order. I'm not completely sure what the right way is to achieve this. At frame construction time we don't know what the previous frame sibling is. We could propagate that down from nsCSSFrameConstructor but that's a lot of work. We could wait for frame insertion or frame append (note that even if you're appending the frame, it might be that your view needs to be inserted into the middle of its siblings) and then shuffle the view to its correct position. Or we could shuffle the view in nsContainerFrame::SyncFrameViewAfterReflow. I need to think about ths some more.
Yeah, I was going to mention that -- since I remember there being another bug that was just about view tree order, even when frame tree order was OK.
Assignee | ||
Comment 11•21 years ago
|
||
> we should really add NS_WARNINGs to places where we knowingly do the wrong
> thing. :(
Nah. We would drown in NS_WARNINGs :-)
Comment 12•21 years ago
|
||
roc, why is it easier to use the frame tree? If the frame and content trees are the same order (as they should be), why not use the already-written content code for doing tree-order comparisons?
Blocks: 217694
Comment 13•21 years ago
|
||
> Nah. We would drown in NS_WARNINGs :-)
Bah. Maybe it would motivate people to do some fixin'? ;)
Assignee | ||
Comment 14•21 years ago
|
||
I guess that right after the frame has been put into the frame tree, we can
fairly easily use my new CompareTreePosition code to figure out whether the view
needs to be shuffled and if so where it should go.
The tricky part is making sure that the pure append case is fast so pageload
doesn't get hurt. I'll see how it goes.
> why not use the already-written content code for doing tree-order comparisons?
Because thinking about three parallel trees at once hurts my brain? :-)
Seriously, you may be right. What's the content tree comparator function called?
Comment 15•21 years ago
|
||
CompareDocumentPosition -- see nsIDOM3Node.idl One other thing. When we move to stylesheets not blocking the parser, then there is a nontrivial chance that a lot of content will in fact follow the not-pure-append path, since we will have a good chunk of the DOM tree constructed before the style sheets come in and trigger a style reresolve...
But it will still be rare that we construct frames without the stylesheets, and we should use a long enough timeout for the case that we do so that we're unlikely to be slower than the network.
Assignee | ||
Comment 17•21 years ago
|
||
Hmm. I like the idea of working over the content tree more than using this actual code (actually nsContentUtils::ComparePositionWithAncestors is the function I want). Lots of QI and dynamic arrays and refcounting, for something that's going to get called a lot. Let me try to write a version that uses nsIContent and is a bit more efficient.
Comment 18•21 years ago
|
||
Layout can't get at nsContentUtils last I checked... should we correct that? Also, be careful when changing from nsIDOMNode to nsIContent -- documents are nodes, but not contents...
Assignee | ||
Comment 19•21 years ago
|
||
Right, and I don't care about documents or attributes or any of the other non-element cases that CompareDocumentPosition tries to handle.
Comment 20•21 years ago
|
||
True. I don't think those checks should be a big perf hit, but if we're worried we may indeed have to hook in at the lower level or write our own version of this...
Assignee | ||
Comment 21•21 years ago
|
||
I have written my own nsIContent version and it's tiny.
Assignee | ||
Comment 22•21 years ago
|
||
*** Bug 217694 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 23•21 years ago
|
||
Here it is. Gets rid of a fair number of XXXs :-). Using the content tree to do the lookup has the advantage that we can find the right position for the view in CreateViewForFrame, even though the frame hasn't been put in the frame tree yet. The content tree approach fails when some of the content is anonymous. Is there actually any ordering on anonymous children? It's unclear what the page load impact might be. InsertOutOfFlowFrames tries to see if the frames can be appended to the containing block first, before going back and looping through all the children. That shoud help.
Comment 24•21 years ago
|
||
> Is there actually any ordering on anonymous children?
Hmm.... There is with XBL, no (eg abs pos things as part of an XBL binding,
being dynamically manipulated (eg display set to "none" and then "block").
Assignee | ||
Comment 25•21 years ago
|
||
What I mean is this: does our content system actually keep track of the order of anonymous children in any way? If yes, how do I get at it? If no, should it?
Comment 26•21 years ago
|
||
Ah. No, it does not. I'm not quite sure whether it should, to be truthful, or even whether it could (given the multifarious ways anonymous content can arise...)
Assignee | ||
Comment 27•21 years ago
|
||
That's fine with me, but if the document order of anonymous elements is undefined, then so is the natural z-order of those elements, so we just have to remember to mark bugs about incorrect z-ordering of anonymous content as WONTFIX :-)
Assignee | ||
Updated•21 years ago
|
Attachment #131013 -
Flags: superreview?(dbaron)
Attachment #131013 -
Flags: review?(dbaron)
Comment 28•21 years ago
|
||
Yeah... I guess that's one argument for using the frame tree instead of the content tree (as long as we are writing a new comparison function)... I sort of suspect that we won't really run into issues with this in XBL, though (since I don't think you can easily create new root content nodes in the binding, and the rest of the binding is not exactly anonymous, and since the original root content nodes will get ordered correctly by frame construction). Other types of anonymous content should really not be getting z-indices.
Attachment #131013 -
Flags: superreview?(dbaron)
Attachment #131013 -
Flags: superreview+
Attachment #131013 -
Flags: review?(dbaron)
Attachment #131013 -
Flags: review+
Assignee | ||
Comment 29•21 years ago
|
||
Fix checked in.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Comment 30•21 years ago
|
||
This checkin may have caused Bug 222468.
Updated•6 years ago
|
Component: Layout: View Rendering → Layout: Web Painting
You need to log in
before you can comment on or make changes to this bug.
Description
•