The default bug view has changed. See this FAQ.

[FIXr]{ib}rel pos inlines not leading to right containing block for abs pos kids

RESOLVED FIXED in mozilla1.8alpha1

Status

()

Core
Layout: R & A Pos
P1
normal
RESOLVED FIXED
15 years ago
9 years ago

People

(Reporter: Florian Hartig, Assigned: bz)

Tracking

(Blocks: 3 bugs)

Trunk
mozilla1.8alpha1
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(3 attachments, 5 obsolete attachments)

(Reporter)

Description

15 years ago
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.9) Gecko/20020311
BuildID:    2002031106

At the URL entered above there should be a "drop up" menu when you point the
mouse over the text items on the grey bar. To do this I tried to use
document.getElementById(elem).style.display = 'block'; where elem has been set
to display : none via css before.
IMHO the method I used is standardized and so this might be a bug.

Reproducible: Always
Steps to Reproduce:
1. Point your browser to the URL given above
2. Move the mouse over the menu items
3. See absolutely nothing happen

Actual Results:  Nothing - in fact that's the problem

Expected Results:  A menu should have popped up right above the menu item your
mose is over

The image used on my example page is intentionally non-existant...

Comment 1

15 years ago
Browser, not engine. Reassigning to DOM Style; cc'ing Boris.

Confirming with Mozilla trunk build 20020330xx WinNT.

As the reporter states, he is doing this:

       document.getElementById(elem).style.display = 'block';

where elem has been set to display : none via css before.


The JavaScript Debugger shows the value 'block' is successfully
assigned in the DOM, but there is no visual response in the browser.

You can experiment by loading the URL and just using this javascript:

 javascript:void(document.getElementById('menu1').style.display='block');

In IE6 it brings up the menu; in Mozilla, it does not.
Note: no errors in the JS Console.
Assignee: rogerl → jst
Status: UNCONFIRMED → NEW
Component: JavaScript Engine → DOM Style
Ever confirmed: true
QA Contact: pschwartau → ian
The problem is the ".menu { display: inline }" rule in the stylesheet.  This
means that the outer menu boxes are inline boxes, whereas the inner boxes (the
ones whose display property is getting toggled) are block boxes.

This puts us into a block-in-inline situation, which is, as usual, very screwy.
 There're various hacks in the CSS frame constructor to handle this, but it
looks like none of them are helping in this case.... over to layout.
Assignee: jst → attinasi
Component: DOM Style → Layout
OS: Windows 2000 → All
QA Contact: ian → petersen
Hardware: PC → All
Summary: document.getElementById(elem).style.display can't be switched → {ib}document.getElementById(elem).style.display can't be switched

Comment 3

15 years ago
Taking.
Assignee: attinasi → waterson

Comment 4

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

Updated

15 years ago
Priority: -- → P4

Updated

15 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → Future
Created attachment 96949 [details]
Testcase

This is a problem with reflow of abs pos elements with auto top but _not_ auto
bottom which are inside a rel pos inline...
Created attachment 96955 [details]
Better testcase

This makes it obvious that the problem is that we are using the parent
blockframe of the rel pos inline as the containing block. This happens to have
an unconstrained height, so we get what we see here (the child div of the
leftmost menu1 is just 7*1e7 pixels down from its parent).
Attachment #96949 - Attachment is obsolete: true
David, what do you think?  Is there a reasonable way we could use the right
containing block here?
I have been experiencing the same problem.

After invoking document.getElementById(its_id).style.display='none'
it is switched to none (I checked that with
alert(document.getElementById(its_id).style.display) and it showed "none") but
it is still visible.

My example page for this bug can be found at:
http://www.marcoos.zwm.punkt.pl/bugzilla/mozilla-style-display.html
Marek, I see no relatively posisioned inline elements on your page.  Therefore
it's a different bug (unless I just missed them).  Please file a bug and cc me, ok?
Summary: {ib}document.getElementById(elem).style.display can't be switched → {ib}rel pos inlines not leading to right containing block for abs pos kids
Blocks: 71150
Blocks: 78880
.
Assignee: waterson → position
Status: ASSIGNED → NEW
Component: Layout → Layout: R & A Pos
Priority: P4 → P2
QA Contact: moied → ian
Target Milestone: Future → ---
Blocks: 93060

Updated

14 years ago
Target Milestone: --- → Future
Blocks: 210746
Blocks: 213556

Updated

14 years ago
Blocks: 202068

Updated

14 years ago
Blocks: 224832
Blocks: 225018
Blocks: 112015
Blocks: 225265
Blocks: 148095
Blocks: 231026
Blocks: 235460
*** Bug 78880 has been marked as a duplicate of this bug. ***
The problems first start when we use the parentReflowState->mCBReflowState as
the containing block in nsHTMLReflowState::InitConstraints...  The problem is
that a rel pos inline is not ever a containing block except for abs pos frames.

The more I think about it, the more implementing this sucks.
Created attachment 142550 [details] [diff] [review]
Partial fix

This makes mCBReflowState be the containing block of _this_ frame, not of the
parent.  That allows us to adjust mCBReflowState for abs pos elements as
needed...

The rest is mostly dealing with fallout and fixing some other random bugs in
ComputeHypotheticalBox.

This still has one problem -- placeholders are not yet placed in
nsPositionedInlineFrame::Reflow when it goes to reflow the abs pos kids.  I'm
not quite sure how to deal with that, and as a result auto offsets on abs pos
things that were initially inlines don't work quite right when the cb is rel
pos.

This patch fixes most of the issues in this bug and its dependancies, other
than that.
Comment on attachment 142550 [details] [diff] [review]
Partial fix

David, what do you think?  This is enough of an improvement to be worth
checking in, imo (for one thing, no more NS_UNCONSTRAINEDSIZE being used as an
offset....)  Does the general approach seem reasonable?  Should I just remove
the copy constructor?
Attachment #142550 - Flags: superreview?(dbaron)
Attachment #142550 - Flags: review?(dbaron)
Blocks: 5016
Created attachment 142622 [details] [diff] [review]
Oops, I had a sign off
Attachment #142550 - Attachment is obsolete: true
Attachment #142622 - Flags: superreview?(dbaron)
Attachment #142622 - Flags: review?(dbaron)
Attachment #142550 - Flags: superreview?(dbaron)
Attachment #142550 - Flags: superreview-
Attachment #142550 - Flags: review?(dbaron)
Attachment #142550 - Flags: review-
Have you run the regression tests?

You should just remove the copy-constructor and copy-assignment operator and
comment in the header file that the compiler-generated ones are OK (or use the
comment from revision 3.31 of the header file).
(Also, I never liked the way mCBReflowState worked -- this is much better.  But
when I fixed bug 143706, the current way was a much easier way to do it.)
(I'm not going to be able to review this before the freeze -- it looks quite
complicated.)
(And, that said, if I could, I'm not sure I'd want it to go in before the freeze.)
Comment on attachment 142622 [details] [diff] [review]
Oops, I had a sign off

Yeah, this is alpha-fodder at this point.  I'll run the regression tests.  I
also just caught another sign error; I'll give the whole patch another
once-over...
Attachment #142622 - Flags: superreview?(dbaron)
Attachment #142622 - Flags: superreview-
Attachment #142622 - Flags: review?(dbaron)
Attachment #142622 - Flags: review-
Blocks: 236966
Created attachment 143908 [details]
Another testcase

This patch doesn't fix that testcase... the problem is that the offsets are
measured differently if the containing block is a positioned inline....  I
probably need to handle this by munging the offsets for cases when that happens
(that is, having mComputedOffsets be the offsets from the padding box as they
are for a block-level containing block and adusting absolutely specified
offsets).
Attachment #142622 - Attachment is obsolete: true
Created attachment 143909 [details]
Real another testcase
Attachment #143908 - Attachment is obsolete: true
Created attachment 143962 [details] [diff] [review]
Patch updated to comments

OK, I removed the constructor and fixed the sign errors and the behavior on
that last testcase I attached.

I also ran the regression tests.  It seems that doing so now requires
approximately 1.7 GB of hard drive space (!) for the rgd files and that some of
Ian's bogus char tests really push the limits of memory usage on my machine
(with 784 MB of RAM).  Also, one of Ian's tests opens a popup window (we should
probably figure out which and remove it).

Anyway, this patch passes.  The only testcases it really changes are some of
Ian's abs-pos-in-rel-pos-inline tests, all for the better.

This patch still isn't quite perfect, because auto offset handling is a bit off
(see XXX comments in the patch), but I'd like to work on that in a separate
patch.
Attachment #143962 - Flags: superreview?(dbaron)
Attachment #143962 - Flags: review?(dbaron)
I tried this patch and it fixed the testcases in bug 224832. Great!

It doesn't fix the scenario I outlined here:
http://bugzilla.mozilla.org/show_bug.cgi?id=237343#c11
That'll be rather hard to fix, but it should wait until after this lands, for sure.

Also, 'right' and 'bottom' should be relative to the edges of the last-in-flow
frame for the element, but aren't ... but I guess that's what you meant by this:
+      // XXXbz we should be taking the in-flows into account too, but
+      // that's very hard.
One way to deal with the last-in-flows and the justification problem would be to
delay reflowing absolute children of positioned inlines until the end of the
reflow of the block containing those inlines.

Although, hey, what if the inlines span a page boundary? :-)
Yeah, the page boundary case is exactly why I didn't even bother trying to
address the in-flow problems for inlines.  Between that and the fact that
depending on how line-breaking works you could have your abs pos element's
containing block width vary by the width of the browser window, I still feel
that what CSS specifies here makes absolutely no sense and that time would be
better spent making the spec sane (and interoperably implementable) rather than
trying to implement it as-is.

As for the issue in bug 237343 comment 11, perhaps that is in fact the right
place to hook into the "post-normal-reflow" layout of our abs pos kids... that
may also fix the XXX comment in my patch about placeholders not being
reflown/positioned yet when we hit CalculateHypotheticalBox.
Blocks: 157526
Comment on attachment 143962 [details] [diff] [review]
Patch updated to comments

>+    NS_ASSERTION(NS_FRAME_GET_TYPE(mFrameType) != NS_CSS_FRAME_TYPE_ABSOLUTE ||
>+                 mCBReflowState->mStyleDisplay->IsPositioned(),
>+                 "Absolutely positioned frame must have positioned containing block!");

It's not obvious to me what prevents this assertion from firing all the time. 
Should there be a precondition about parentReflowState?  Other than that,
r+sr=dbaron.
Attachment #143962 - Flags: superreview?(dbaron)
Attachment #143962 - Flags: superreview+
Attachment #143962 - Flags: review?(dbaron)
Attachment #143962 - Flags: review+
> Should there be a precondition about parentReflowState?

There is an early return out of this function if parentReflowState is null... 
So by the time we get to that assert we know that we _do_ have a parent reflow
state.  The assert would fire if we have a frame with a display struct that
returns true for IsAbsolutelyPositioned() and has a parent whose display struct
doesn't return true for IsPositioned().  Come to think of it, that may happen
for fixed-pos frames; I'll look into that.  But except for that case, I think
this assert is correct...
Assignee: core.layout.r-and-a-pos → bzbarsky
Priority: P2 → P1
Summary: {ib}rel pos inlines not leading to right containing block for abs pos kids → [FIXr]{ib}rel pos inlines not leading to right containing block for abs pos kids
Target Milestone: Future → mozilla1.8alpha
Created attachment 146927 [details] [diff] [review]
Patch that I checked in

I just removed the assert; it was firing for fixed-pos frames and for abs pos
table frames (since the containing block was the outer frame, which was not
positioned) and could have possibly fired in some other edge cases like that...
I decided that it wasn't worth trying to figure out when it should "really"
fire.

I also added two lines to enforce that the computed width/height of an abs pos
frame is nonnegative (it was ending up negative if it was auto and the offsets
were non-auto and "left + right" (respectively "top + bottom") was bigger than
the available width (respectively height)).
Attachment #143962 - Attachment is obsolete: true
Checked in. Marking fixed, though the containing block is not quite "right"
still.  Bug 5016 should track the remaining work.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED

Updated

12 years ago
Depends on: 300816
Depends on: 419072
You need to log in before you can comment on or make changes to this bug.