Last Comment Bug 52916 - [FLOAT] Floating text overlapping max-width elements
: [FLOAT] Floating text overlapping max-width elements
Status: RESOLVED FIXED
[Hixie-P4] (py8ieh:examine) potential...
: css2, testcase, topembed-
Product: Core
Classification: Components
Component: Layout: Floats (show other bugs)
: Trunk
: All All
: P2 major with 1 vote (vote)
: Future
Assigned To: David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
: Hixie (not reading bugmail)
Mentors:
: 69915 249774 (view as bug list)
Depends on:
Blocks: float 104166
  Show dependency treegraph
 
Reported: 2000-09-16 05:24 PDT by Karl Ove Hufthammer
Modified: 2008-04-15 18:28 PDT (History)
18 users (show)
asa: blocking1.6a-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (1.04 KB, text/html)
2000-10-07 14:33 PDT, Ronald Tilby
no flags Details
opera screenshot (21.45 KB, image/png)
2001-07-27 00:15 PDT, basic
no flags Details
fix? (829 bytes, patch)
2002-01-30 20:30 PST, Chris Waterson
dbaron: review+
bzbarsky: superreview+
Details | Diff | Splinter Review
another testcase (resize the window to see the effect) (556 bytes, text/html)
2002-03-20 14:12 PST, Alexandru Savulov
no flags Details
Screenshot of the bug (95.21 KB, image/png)
2003-11-24 13:10 PST, Radosław Szkodziński
no flags Details

Description Karl Ove Hufthammer 2000-09-16 05:24:49 PDT
See http://home.no.net/huftis/artiklar/tabell.html for an example. The TOC menu 
is overlapping the text.

On this page, all paragraphs have a 'max-width' set, and use 'margin: auto'. 
The 'TOC' has a 'float: right' set, and is overlapping the paragraphs (it 
should of course not).

If I change 'max-width' to 'width', everything works correctly (the TOC doesn't 
overlap), so this is probably a problem computing remaining space when elements 
have a 'max-width'. This is a *very* serious bug.
Comment 1 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2000-09-16 10:06:42 PDT
Layout, not Style System.
Comment 2 R.K.Aa. 2000-09-16 19:23:02 PDT
This is not a blocker. It does not block development or testing.
Lowering severity.
Comment 3 Chris Petersen 2000-10-02 11:51:54 PDT
I can see this issue with the Sept 29th build. Marking new.
Comment 4 buster 2000-10-06 13:28:38 PDT
a reduced test case would go a long way towards getting this examined before NS6
RTM.  Setting to P2.
Comment 5 Ronald Tilby 2000-10-07 14:33:50 PDT
Created attachment 16455 [details]
testcase
Comment 6 buster 2000-10-13 17:08:11 PDT
P2 bugs -> future
Comment 7 Hixie (not reading bugmail) 2000-12-11 16:14:54 PST
Upon managerial request, adding the "testcase" keyword to 84 open layout bugs that
do not have the "testcase" keyword and yet have an attachement with the word
"test" in the description field. Apologies for any mistakes.
Comment 8 basic 2001-07-27 00:15:50 PDT
Created attachment 43771 [details]
opera screenshot
Comment 9 Chris Waterson 2001-07-27 11:09:08 PDT
taking.
Comment 10 Robin Lionheart 2001-08-03 04:25:49 PDT
Also see the test case for bug 69915.
Comment 11 Marc Attinasi 2001-10-22 17:17:26 PDT
Taking, marking topembed: this was spin-off from bugscape bug 8601.

Chris, I'm taking this to work on - hope you do not mind. I'm hoping to get out
Float story in shape in the next few weeks.
Comment 12 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2001-10-22 17:21:37 PDT
Is this really related to something on a real website?  Few browsers support
'max-width'.
Comment 13 Marc Attinasi 2001-10-22 18:01:02 PDT
Ehem - sorry. I was mistaken, not careful enough, etc.

The problem I was trying to find a bug for LOOKS just like this, but the markup
is very different. Returning this bug to it's original state, opening a new one
(until I can find a proper dup).
Comment 14 Marc Attinasi 2001-10-22 18:01:37 PDT
re-accepting for waterson (I mistakenly took it, now giving it back).
Comment 15 Asa Dotzler [:asa] 2001-12-03 11:08:57 PST
Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 
(you can query for this string to delete spam or retrieve the list of bugs I've 
moved)
Comment 16 Asa Dotzler [:asa] 2001-12-03 17:18:45 PST
don't move bugs that are in the 1.0 dependency tree. sorry.

Comment 17 Chris Waterson 2002-01-29 20:22:21 PST
I've looked into this a bit, and the problem appears to be that we're
determining that there's enough space to flow the line _before_ applying the
left margin. Specifically, we're not translating the space manager's coordinate
system, so the space available for the <p> is computed as if there were no
margin. I'm guessing that the margin is computed and applied later when the
max-width constraint is applied, but I need to understand this more thoroughly...
Comment 18 Chris Waterson 2002-01-30 20:30:59 PST
Created attachment 67208 [details] [diff] [review]
fix?

We're not recomputing the block's side margins in the case where our width is
automatic, but constrained by max-width. I think we need to do this, but I'm
not terribly familiar with this code.
Comment 19 Christian Reis 2002-02-12 19:46:56 PST
Can we get review moving on this now that we have a patch (thanks Waterson)? 
Comment 20 Kevin McCluskey (gone) 2002-02-27 13:55:33 PST
Reassigning to Alex.
Comment 21 Marc Attinasi 2002-02-28 16:14:43 PST
*** Bug 69915 has been marked as a duplicate of this bug. ***
Comment 22 karnaze (gone) 2002-03-12 14:46:31 PST
topembed+, nsbeta1+, m1.0
Comment 23 Alexandru Savulov 2002-03-12 16:44:52 PST
I will analyse/review the patch Waterson prepared, so if everything is ok and i
can get another review and a super review we can check it in. Otherwise we
shouldn't bother to resolve this soon.

Opera and IE act different with this test case. Seems to me that IE does not
even work with the max-width property. If we do something here, than we should
only prevent the text from overlapping. Trying to emulate here IE would not make
any sense. I also doubt that the authors will use features that do not work on IE.

Comment 24 Karl Ove Hufthammer 2002-03-13 00:50:01 PST
> Seems to me that IE does not
> even work with the max-width property.

FYI: This is correct. IE doesn't, and has never supported 'max-width', unfortunately.
Comment 25 Kevin McCluskey (gone) 2002-03-20 11:13:43 PST
Marking nsbeta1-, topembed- since Engineers are overloaded with higher priority
bugs and there isn't likely to be much content which makes use of the max-width
property since IE doesn't support it. Pushing to future-P1. We will pull this
bug back after we work through the other bugs.
Comment 26 Alexandru Savulov 2002-03-20 14:12:01 PST
Created attachment 75260 [details]
another testcase (resize the window to see the effect)
Comment 27 Syd Logan 2002-03-21 22:19:43 PST
adt agrees nsbeta1-
Comment 28 Chris Petersen 2002-08-01 11:44:29 PDT
Still reproduces in the OS X 2002-07-31-05 NB.
Comment 29 Karl Ove Hufthammer 2002-12-12 02:08:56 PST
I can confirm that this bug is still present in Moz 1.2.1, using
http://bugzilla.mozilla.org/attachment.cgi?id=16455&action=view.
Comment 30 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2003-03-30 14:16:09 PST
->Layout:Floats
Comment 31 Robin Lionheart 2003-05-09 12:40:42 PDT
Still present in 1.3. The body text overlaps the floating menu of the front page
of my test suite at http://www.robinlionheart.com/stds/html4/.
Comment 32 Kob 2003-05-28 22:08:29 PDT
Is this the same bug ?  Both menu at left and floaters (?) at right overlap text
at http://research.microsoft.com/projects/ilx/fsharp.htm
Seen with 
1. Mozilla/5.0 (Windows; U; WinNT4.0; en-US; rv:1.4b) Gecko/20030516 Mozilla
Firebird/0.6
2. Mozilla/5.0 (Windows; U; WinNT4.0; en-US; rv:1.3) Gecko/20030312
Comment 33 James Kilfiger 2003-10-21 19:35:14 PDT
I've browsing for a few weeks with "p {max-width: 50em}" in userContent.css, and
this bug does appear in many pages, including the firebird home-page. Waterson's
patch makes sence to me, a good patch for alpha? => nominating. 
Comment 34 James Kilfiger 2003-11-12 16:28:37 PST
There is an unoffical build by scragz, including the patch to this bug at
http://forums.mozillazine.org/viewtopic.php?t=33958
Comment 35 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2003-11-12 17:01:39 PST
Comment on attachment 67208 [details] [diff] [review]
fix?

This patch looks ok to me, but I think we might want to move this out a level
or two of braces (although I'm not sure)
Comment 36 Boris Zbarsky [:bz] 2003-11-12 20:07:18 PST
Comment on attachment 67208 [details] [diff] [review]
fix?

sr=bzbarsky

Hoisting this out would indeed make the code clearer and less prone to errors
like that, I think.

David, I assume you will be checking this in?
Comment 37 Radosław Szkodziński 2003-11-24 13:10:04 PST
Created attachment 136247 [details]
Screenshot of the bug

The patch doesn't still fix the bug on this page:
http://www.mozilla.org/products/firebird/

The testcases look properly though.

I'm using Nova build from 2003-11-24 with the patch in.
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031124
Firebird/0.7+ (Nova: MNG,DOMi)

Link: http://forums.mozillazine.org/viewtopic.php?p=274854
Comment 38 Boris Zbarsky [:bz] 2003-11-24 13:20:47 PST
David, would you say this is worth trying for 1.6b?
Comment 39 Radosław Szkodziński 2003-11-24 13:38:35 PST
Disregard the last comment. It seems Nova didn't have the patch. :(
Sorry for spam.
Comment 40 Mike Tigas 2003-11-24 13:56:26 PST
AstralStorm, I did have the patch.  When I was answering your question on the
MozillaZine thread, we were both having a bit of miscommunication.  Your comment
#37 is still valid, as far as I know.

But in regard to that Firebird page:  I have snippets of importance from the
HTML and CSS of that page and the Mozilla.org style sheet quoted here:

http://forums.mozillazine.org/viewtopic.php?p=274916#274916

>The DD element on that page is set to "nowrap" so it shouldn't
>wrap the text.  Isn't it?

Am I correct in guessing that that's why the Firebird product page displays that
behavior?

I'll re-iterate that my build _does_ have the patch (attachment 67208 [details] [diff] [review]).  The
testcases display fixed behavior on my build.
Comment 41 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2003-11-24 14:07:34 PST
Actually, moving it out a level or two wouldn't change anything since
CalculateBlockSideMargins would return immediately in all the other cases.
Comment 42 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2003-11-24 14:09:43 PST
comment 37, comment 39, and comment 40 have nothing to do with *this* bug. 
Please discuss that issue in bug 225645.  It's a bug in the site, not a bug in
the browser.
Comment 43 Mike Tigas 2004-01-05 20:42:29 PST
I'd been using the patch (attachment #3 [details] [diff] [review]) for this in my builds for a while.

The fix for bug 227819 with the change to
layout/html/base/src/nsHTMLReflowState.cpp conflicts with the fix for this bug,
as far as I can tell.

It's the change around line 1971:

http://tinderbox.mozilla.org/bonsai/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/layout/html/base/src&command=DIFF_FRAMESET&file=nsHTMLReflowState.cpp&rev1=1.186&rev2=1.187&root=/cvsroot

I removed the patch for this bug to fix bustage.  Just wanted to bring this up, FYI.
Comment 44 Boris Zbarsky [:bz] 2004-01-05 21:04:16 PST
To David to decide whether he wants to just land that as-is.
Comment 45 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-01-17 22:07:42 PST
Fix checked in to trunk, 2004-01-17 22:06 -0800.
Comment 46 Martijn Wargers [:mwargers] (not working for Mozilla) 2004-06-15 09:47:41 PDT
Could this have caused bug 244932? Backing out the patch for this bug makes the
testcase for bug 244932 work.
Comment 47 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-07-04 18:36:49 PDT
*** Bug 249774 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.