Closed
Bug 59200
Opened 24 years ago
Closed 23 years ago
errors wrapping floats with width:auto [FLOAT]
Categories
(Core :: Layout, defect, P3)
Tracking
()
VERIFIED
FIXED
mozilla0.9.1
People
(Reporter: theamigo, Assigned: waterson)
References
(Blocks 1 open bug, )
Details
(Keywords: css1, testcase, Whiteboard: [Hixie-P1] (this is the worst floater bug) WG (py8ieh:verify new bugs were opened))
Attachments
(12 files)
930 bytes,
text/html
|
Details | |
622 bytes,
text/html
|
Details | |
735 bytes,
text/html
|
Details | |
19.56 KB,
image/png
|
Details | |
6.26 KB,
text/html
|
Details | |
381 bytes,
text/html
|
Details | |
1.10 KB,
patch
|
Details | Diff | Splinter Review | |
92.51 KB,
patch
|
Details | Diff | Splinter Review | |
21.15 KB,
patch
|
Details | Diff | Splinter Review | |
21.27 KB,
patch
|
Details | Diff | Splinter Review | |
249 bytes,
text/html
|
Details | |
61.17 KB,
image/gif
|
Details |
From Bugzilla Helper: User-Agent: Mozilla/4.0 (compatible; MSIE 5.0; Windows 98; DigExt) BuildID: 2000101014 When the window is just narrower than the combined widths of boxes one and two, box three is displayed in the wrong location. When box two is wrapped, box three is displayed above and to the right of box two. This is incorrect behavior according to CSS1 section 4.1.4 rule #2: "The left outer edge of a left floating element must be to the right of the right outer edge of every earlier (in the HTML source) left-floating element or the top of the former must be lower than the bottom of the latter. Analogously for right floating elements." When viewed in IE5, the page is rendered properly. Reproducible: Always Steps to Reproduce: 1. Load page in M18 2. Adjust window width to just narrower than combined width of boxes one and two. 3. Observer placement of box three Actual Results: Box three is above box two and not to the right of box two. Expected Results: Box three should be either completely below or completely to the right of box two.
Comment 1•24 years ago
|
||
Interesting. Removing the <nobr> and </nobr> tags from the source makes it lay out the boxes correctly on linux trunk build 2000110421. Though the sizing of the divs is still a bit weird...
Comment 2•24 years ago
|
||
Comment 3•24 years ago
|
||
I shall attach a series of test cases showing the various bugs present.
Status: UNCONFIRMED → NEW
Ever confirmed: true
QA Contact: lorca → ian
Summary: float style div wraps wrong → errors wrapping floats with width:auto
Comment 4•24 years ago
|
||
Comment 5•24 years ago
|
||
Comment 6•24 years ago
|
||
triaging...
Assignee: clayton → buster
Summary: errors wrapping floats with width:auto → errors wrapping floats with width:auto [FLOAT]
Comment 7•24 years ago
|
||
Comment 8•24 years ago
|
||
The correct behavior can be seen on build 2000110804 on Windows 98 when you first maximize the window, reload the broken page, and finally narrow down your window.
Comment 9•24 years ago
|
||
It turns out this bug completely kills the use of floats for layout -- see, for example, the page I will attach shortly: it should have no overlap, and no overflow in the floats. Oops. So much for fluid document layouts. This is _the_ floater bug I would recommend fixing for mozilla0.9.
Severity: normal → major
Keywords: mozilla1.0,
nsbeta1
Comment 10•24 years ago
|
||
Comment 12•24 years ago
|
||
Note that I am assuming that the issue that floats which have wrapped end up having their width decided by the space remaining on the *previous* `line' is the main bug here. The issue that floats use remaining space rather than being exclusively based on their contents may or may not be correct depending on the outcome of a current thread in the CSS working group. If this would be clearer with a new bug report, I would be happy to open one. IMHO this is the most serious floater bug in Mozilla at the moment. See http://bugzilla.mozilla.org/showattachment.cgi?attach_id=21033 for a particularly explicit example of why.
Whiteboard: [Hixie-P1] (this is the worst floater bug) WG
Comment 13•23 years ago
|
||
Assignee | ||
Comment 16•23 years ago
|
||
So it looks like when troy first implemented BRS_SHRINKWRAPWIDTH, he made PostPlaceLine() forcibly take up the full amount of space. I'm not sure this is necessary. Removing this code fixes the simplest test cases in this bug without regressing bug 38157 (the 2nd and 3rd attachments, made by Hixie on 11/12/00 13:55 and 13:59) http://bugzilla.mozilla.org/showattachment.cgi?attach_id=19135 http://bugzilla.mozilla.org/showattachment.cgi?attach_id=19136 Nevertheless, it does *not* appear to fix the originally reported bug; nor does it fix the original test case, or the dread ``Da Mow Mow Portal''. I also just noticed that we appear to layout http://www.w3.org/Jigsaw/ wrong: note the position of the topmost graphic and the tag-line, probably worth another bug. cc'ing smart people who probably know more about shrink-wrap than I do.
Status: NEW → ASSIGNED
Assignee | ||
Comment 17•23 years ago
|
||
Assignee | ||
Comment 18•23 years ago
|
||
FWIW, the last patch makes it through the layout regression tests cleanly.
Comment 19•23 years ago
|
||
Chris: the damowmow portal triggers approximately a gazillion different floater bugs, so while your fix should probably make a difference, don't expect the page to magically start working either.
Assignee | ||
Comment 20•23 years ago
|
||
Assignee | ||
Comment 21•23 years ago
|
||
There are at least two more obvious bugs in the floater code; the above patch fixes one of them (for the most part). Specifically, as we advance mY to find space for the floater, we need to reflow the floater in the new space. I've partially fixed this by combining the floater's reflow with the floater's placement, renaming PlaceFloater() to FlowAndPlaceFloater(). This method reflows the floater, and then attempts to place it, looping until it the floater is placed. There is still a bit of work to be done here, as CanPlaceFloater() will sometimes advance the reflow state's mY, which might necessitate more reflow. The other bug that I discovered is that we need to obey the floated frame's maxElementSize, and never let the floater's width fall below that value.
Assignee | ||
Comment 22•23 years ago
|
||
Assignee | ||
Comment 23•23 years ago
|
||
Okay, so I was wrong about CanPlaceFloater(); it only needs to manipulate mY so it can ``peer'' down into the bands below. So this changes two major things about floaters: 1. What was previously a two-call lockstep process (reflow the floater, then try to place it) has been combined into one call, nsBlockReflowState::FlowAndPlaceFloater(), which will iteratively reflow the floater until we find a band where the floater fits. 2. For auto-width floaters, ensures that the floater does not get flowed at a width less than its maxElementSize. This was obvious in the above test cases: if you shrank the window enough, you'd get something that looked like: +----------+ | Supercalifragiliticexpialidocius +----------+ instead of: +----------------------------------+ | Supercalifragiliticexpialidocius | +----------------------------------+ Another issue that I'd like to address (although I'm not sure if I should address it in a different bug), is the violation of 9.5.1 rule [2]. I think that this may be easy to do if the block's floater cache is sorted in the same order as the elements. (I'm not sure if that's actually the case; maybe it's already done.) If it _were_ the case, then we could simply advance mY to the mY of the previous floater, and start looking for space there.
Assignee | ||
Comment 24•23 years ago
|
||
FWIW, this passes the block & table layout regression tests, as well as dbaron's floater tests (at least, it passes them as well as we used to).
Assignee | ||
Comment 25•23 years ago
|
||
Assignee | ||
Comment 26•23 years ago
|
||
The last patch adds a member variable |mLastFloaterY| to the block reflow state, which maintains the y-coordinate of the last floater that we placed. This allows us to enforce 9.5.1 rule [2], because we'll start the search for a floater's home on the same line where the last floater was placed.
Assignee | ||
Comment 27•23 years ago
|
||
Comment 28•23 years ago
|
||
Comment 29•23 years ago
|
||
Had another look at the browser window. Actually, the floats are _not_ missing, they are just sitting below the bigger graphics, i.e., the 'Hosting' and 'Boilerplate' floats are placed next to the other 'W3C', 'Bookmarklets', etc. Is this behavior also deemed correct? They appear to be really _floating_ in a literal sense.
Assignee | ||
Comment 30•23 years ago
|
||
Yes, I realized the same thing. I'll need to take the test case apart to see what's happening. My first inclination is to say, ``probably not the right thing!''
Comment 31•23 years ago
|
||
Your patch makes a significant difference! I tried viewing this dreaded Da Mow Mow Portal without your patch and it was awful. Others cc:ed on the bug who have regular builds can point and click to see for themselves. No way you will get the screenshots that I attached! http://bugzilla.mozilla.org/showattachment.cgi?attach_id=21033 Since it makes the product so much better and you reported to have passed the regressions tests, my r=rbs applies if you want to checkin. If it goes in the tree now (before PDT locks you out for the months to come), much testing can still be made. And as Hixie warned at the beginning, his Da Mow Mow Portal is an extreme test case. Other simpler cases can already benefit fully from your improvements. You can refine in subsequent iterations.
Comment 32•23 years ago
|
||
sr=attinasi - these changes are great!
Comment 33•23 years ago
|
||
>If it goes in the tree now (before PDT locks you out for the months to come), PDT isn't locking anyone out of the cvs.mozilla.org trunk for months to come. Talk to drivers@mozilla.org if you want to land something moderately scary before mozilla1.0 (which we still want to be this year, but which will not be in only a few more milestones). /be
Assignee | ||
Comment 34•23 years ago
|
||
Verified this does not impact page load tests. Before change, I get 856msec (average) on jrgm's tests; afterwards, I get 857msec.
Assignee | ||
Comment 35•23 years ago
|
||
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 36•23 years ago
|
||
VERIFIED that the original test case is now FIXED. I will open new bugs for the bugs still visible on: http://bugzilla.mozilla.org/showattachment.cgi?attach_id=19134 http://bugzilla.mozilla.org/showattachment.cgi?attach_id=19135 http://bugzilla.mozilla.org/showattachment.cgi?attach_id=19136
Status: RESOLVED → VERIFIED
Whiteboard: [Hixie-P1] (this is the worst floater bug) WG → [Hixie-P1] (this is the worst floater bug) WG (py8ieh:verify new bugs were opened)
Comment 37•23 years ago
|
||
FYI: the bugs are (respectively) bug 82313, bug 82314, and bug 82315.
Comment 38•23 years ago
|
||
I have this little theory that converting a resize reflow in a block with floaters to a style change reflow (i.e., total reflow :-) will fix these remaining bugs (albeit in an expensive manner...). But that is just a conjecture with no investigation to back it up. No bug filed for the dreaded Da Mow Mow testcase? Does this mean, you are happy with the currently much improved rendering?
Comment 39•23 years ago
|
||
For all intents and purposes, http://www.damowmow.com/portal/ now renders correctly in Mozilla (for which I am eternally grateful, Chris!). If there are other bugs on that page, they are covered by those three bugs and the other bugs marked as blocking bug 78094.
SPAM. HTML Element component deprecated, changing component to Layout. See bug 88132 for details.
Component: HTML Element → Layout
You need to log in
before you can comment on or make changes to this bug.
Description
•