iframe (and some form elements) & CSS 'p:first-line' don't play nice together

RESOLVED WORKSFORME

Status

()

RESOLVED WORKSFORME
17 years ago
15 years ago

People

(Reporter: bugzilla, Unassigned)

Tracking

({regression, testcase})

Trunk
regression, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [CSS1-2.3] [patch])

Attachments

(5 attachments, 1 obsolete attachment)

(Reporter)

Description

17 years ago
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 :)
(Reporter)

Comment 1

17 years ago
Created attachment 74724 [details]
test case: the iframe overwrites the text in the top left of the window

This test case validates at w3.
(Reporter)

Updated

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

Comment 2

17 years ago
Confirming on Linux 2002031721. Weird...
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: SunOS → All
Hardware: Sun → All

Comment 3

17 years ago
Created attachment 74727 [details]
Testcase without the first-line rule to show the difference.

Comment 4

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

17 years ago
Created attachment 74729 [details]
Testcase #3

It seems like the :first-line selector makes the iframe reset its position to
0,0

Updated

17 years ago
Keywords: testcase
Whiteboard: [CSS1-2.3]

Comment 6

17 years ago
Changing QA contact
QA Contact: petersen → moied

Updated

17 years ago
Priority: -- → P3
Target Milestone: --- → Future
(Reporter)

Comment 7

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

Keywords: regression
.
Assignee: attinasi → block-and-inline
Component: Layout → Layout: Block & Inline
Priority: P3 → --
QA Contact: moied → ian
Target Milestone: Future → ---

Comment 9

16 years ago
*** Bug 209489 has been marked as a duplicate of this bug. ***
Created attachment 127573 [details]
Testcase #4
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]
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

16 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.)
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.
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?
Created attachment 127616 [details] [diff] [review]
Patch rev. 2
Attachment #127575 - Attachment is obsolete: true

Updated

16 years ago
Attachment #127575 - Flags: review?(dbaron)

Updated

16 years ago
Attachment #127616 - Flags: review?(dbaron)
(Reporter)

Updated

16 years ago
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.
Hmm... the testcases in this bug seem to work for me...

Comment 22

15 years ago
WFM: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031119

Comment 23

15 years ago
This bug worksforme with Windows Seamonkey builds 2004020909
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.