Closed
Bug 131698
Opened 22 years ago
Closed 21 years ago
iframe (and some form elements) & CSS 'p:first-line' don't play nice together
Categories
(Core :: Layout: Block and Inline, defect)
Core
Layout: Block and Inline
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: bugzilla, Unassigned)
References
Details
(Keywords: regression, testcase, Whiteboard: [CSS1-2.3] [patch])
Attachments
(5 files, 1 obsolete file)
If I have an IFRAME near the first line of a paragraph, then seemingly any CSS rule containing 'p:first-line' will cause the IFRAME to be placed in the top left of the window (exactly in the top left, overwriting anything else that should be there). I have tried with many combinations of number of iframes, how they are positioned, and css attributes for the p:first-line style. If I have more than 1 iframe, they are all placed on top of each other in the top left of the window. If the iframe is just at the start of a <p>, or after a small amount of text in a <p>, or if the <p> and the iframe are within table cells, the iframe is still displayed in the top left of the window. I have tried margin, border, font-weight and text-decoration as the styles to apply to the 'p:first-line', and in all cases the iframe is displayed in the top left of the window. Note that I'm not a CSS or html expert at all, but I *think* what I'm trying to do is valid :)
Attachment #74724 -
Attachment description: test case: the iframe overwrites the test in the top left of the window → test case: the iframe overwrites the text in the top left of the window
Confirming on Linux 2002031721. Weird...
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: SunOS → All
Hardware: Sun → All
It helps if you put a closing </p> before the iframe-tag. But that doesn't explain the difference between the two testcases.
Comment 5•22 years ago
|
||
It seems like the :first-line selector makes the iframe reset its position to 0,0
Updated•22 years ago
|
Whiteboard: [CSS1-2.3]
Updated•22 years ago
|
Priority: -- → P3
Target Milestone: --- → Future
2 extra things: a) Mozilla 0.7 did not show this problem, and b) I think that combobox/select boxes also suffer from the same problem as iframes: i.e. they get repositioned to (0,0) if they are on a line with a p::first-line css setting
Updated•22 years ago
|
Keywords: regression
Comment 8•21 years ago
|
||
.
Assignee: attinasi → block-and-inline
Component: Layout → Layout: Block & Inline
Priority: P3 → --
QA Contact: moied → ian
Target Milestone: Future → ---
Comment 9•21 years ago
|
||
*** Bug 209489 has been marked as a duplicate of this bug. ***
Comment 10•21 years ago
|
||
Comment 11•21 years ago
|
||
The reason this bug occurs is this optimization: void nsContainerFrame::PositionChildViews(nsIPresContext* aPresContext, nsIFrame* aFrame) { nsFrameState frameState; aFrame->GetFrameState(&frameState); if (! ((frameState & NS_FRAME_HAS_CHILD_WITH_VIEW) == NS_FRAME_HAS_CHILD_WITH_VIEW)) { return; } ... position the views ... The problem is that the NS_FRAME_HAS_CHILD_WITH_VIEW bit has not been setup correctly on the nsFirstLineFrame so we will not reach the code that do the view positioning. I have a patch that seems to fix the problem...
Whiteboard: [CSS1-2.3] → [CSS1-2.3] [patch]
Comment 12•21 years ago
|
||
Comment 13•21 years ago
|
||
There is still some flicker when the IFRAME is not on the first line (load Testcase #4 and make the window narrow) but I believe that is a separate bug.
Updated•21 years ago
|
Attachment #127575 -
Flags: review?(dbaron)
Could you use the new functions (|GetStateBits| and |AddStateBits|) rather than the old ones? Also, you need to check NS_FRAME_HAS_VIEW on the child as well. Finally, the loop would probably be better written as a while loop, and without modifying the parameter (which is generally unclear), i.e., with a new variable (perhaps just |f|). (|ReparentFrame| and |MoveChildrenTo| are looking more and more similar. But probably it's still better to keep them separate.)
Comment 15•21 years ago
|
||
Will do, but regarding NS_FRAME_HAS_VIEW on the child - isn't it possible that the child does not have a view but have a grandchild that has one and thus it will have NS_FRAME_HAS_CHILD_WITH_VIEW without being the one with the view? or does NS_FRAME_HAS_VIEW propagate upward too?
I think you need to check if *either* is set -- NS_FRAME_HAS_CHILD_WITH_VIEW really means descendant, but I don't think it includes self, so I think (maybe I'm wrong, though) that a frame with a view and no descendants with views has NS_FRAME_HAS_VIEW but not NS_FRAME_HAS_CHILD_WITH_VIEW.
Comment 17•21 years ago
|
||
OK, got it, thanks. There is also the question if one should propagate NS_FRAME_HAS_CHILD_WITH_VIEW to a frame that has a view of its own. Looking at nsIFrame::SetView() I think so. That would cover the case the view goes away for some reason. In other words, we should look for either flag on the child but only NS_FRAME_HAS_CHILD_WITH_VIEW on the parents. BTW, shouldn't MoveChildrenTo() propagate the flag to all the parents?
Comment 18•21 years ago
|
||
Attachment #127575 -
Attachment is obsolete: true
Updated•21 years ago
|
Attachment #127575 -
Flags: review?(dbaron)
Updated•21 years ago
|
Attachment #127616 -
Flags: review?(dbaron)
Summary: iframe & CSS 'p:first-line' don't play nice together → iframe (and some form elements) & CSS 'p:first-line' don't play nice together
Comment on attachment 127616 [details] [diff] [review] Patch rev. 2 >@@ -11875,6 +11875,21 @@ > { > aPresContext->ReParentStyleContext(aFrame, aParentStyleContext); > aFrame->SetParent(aNewParentFrame); >+ >+ // Set NS_FRAME_HAS_CHILD_WITH_VIEW on the parents if needed, bug 131698. They're ancestors, not parents. (Yes, parent and child are currently used in the code a lot where ancestors and descendants are meant. That doesn't mean we should continue to do it.) >+ if (!(aNewParentFrame->GetStateBits() & NS_FRAME_HAS_CHILD_WITH_VIEW) && >+ (aFrame->GetStateBits() & >+ (NS_FRAME_HAS_VIEW | NS_FRAME_HAS_CHILD_WITH_VIEW))) { >+ // aFrame or one if its descendants has a view. >+ aNewParentFrame->AddStateBits(NS_FRAME_HAS_CHILD_WITH_VIEW); >+ >+ // Propagate to all parents, we are done if we find one with the bit set. >+ nsIFrame* parent = aNewParentFrame->GetParent(); You're just pulling the first iteration of the loop out of the loop here, which just means extra code. I'd think you could simplify the whole thing to what's below. >+ while (parent && !(parent->GetStateBits() & NS_FRAME_HAS_CHILD_WITH_VIEW)) { >+ parent->AddStateBits(NS_FRAME_HAS_CHILD_WITH_VIEW); >+ parent = parent->GetParent(); >+ } >+ } The simplification (I think) would be: if (aFrame->GetStateBits & (NS_FRAME_HAS_VIEW | NS_FRAME_HAS_CHILD_WITH_VIEW)) for (nsIFrame *f = aNewParentFrame; f && !(f->GetStateBits() & NS_FRAME_HAS_CHILD_WITH_VIEW); f = f->GetParent()) f->AddStateBits(NS_FRAME_HAS_CHILD_WITH_VIEW); > There is also the question if one should propagate NS_FRAME_HAS_CHILD_WITH_VIEW > to a frame that has a view of its own. Looking at nsIFrame::SetView() I think so. > That would cover the case the view goes away for some reason. > In other words, we should look for either flag on the child but only > NS_FRAME_HAS_CHILD_WITH_VIEW on the parents. NS_FRAME_HAS_CHILD_WITH_VIEW is an optimization used by nsContainerFrame::PositionChildViews. > BTW, shouldn't MoveChildrenTo() propagate the flag to all the parents? MoveChildrenTo *may* operate on the assumption that it's moving the children between siblings. I might be wrong, though. And whatever it is should probably be documented sometime...
Attachment #127616 -
Flags: review?(dbaron) → review-
> NS_FRAME_HAS_CHILD_WITH_VIEW is an optimization used by
> nsContainerFrame::PositionChildViews.
I meant to add that yes, it should be set.
Comment 21•21 years ago
|
||
Hmm... the testcases in this bug seem to work for me...
Comment 22•21 years ago
|
||
WFM: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031119
Comment 23•21 years ago
|
||
This bug worksforme with Windows Seamonkey builds 2004020909
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•