Last Comment Bug 252771 - Unnecessary horizontal scrollbar displayed
: Unnecessary horizontal scrollbar displayed
Status: RESOLVED FIXED
: regression, testcase
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: x86 All
: -- normal (vote)
: ---
Assigned To: Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
:
Mentors:
http://www.belgeo.be
: 254690 257327 272960 319944 (view as bug list)
Depends on:
Blocks: 247938
  Show dependency treegraph
 
Reported: 2004-07-23 07:52 PDT by omg itsh
Modified: 2005-12-12 03:21 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Testcase #1 (518 bytes, text/html)
2004-07-23 10:34 PDT, Mats Palmgren (:mats)
no flags Details
fix (11.56 KB, patch)
2004-07-23 20:45 PDT, Robert O'Callahan (:roc) (Exited; email my personal email if necessary)
dbaron: review+
dbaron: superreview+
Details | Diff | Review

Description omg itsh 2004-07-23 07:52:14 PDT
User-Agent:       Mozilla/5.0 (compatible;MSIE 5.5; GNU/Linux)
Build Identifier: Mozilla/5.0 (compatible;MSIE 5.5; GNU/Linux) (from FreeBSD ports)

mozilla used to display http://www.belgeo.be (best mapper for belgium, used by
thousands of people)'s input fields correctly, but now it displays a useless
horizontal scrollbar in the way of the lower input fields.

Reproducible: Always
Steps to Reproduce:
1. connect to http://www.belgeo.be
2. look for an address
Actual Results:  
you have to crawl thru the useless horizontal scroll bar that came up after 1.7.

Expected Results:  
don't display the horizontal bar

belgeo.be is #1 online mapper for belgium.
Comment 1 Mats Palmgren (:mats) 2004-07-23 10:34:56 PDT
Created attachment 154123 [details]
Testcase #1
Comment 2 Mats Palmgren (:mats) 2004-07-23 10:38:17 PDT
Confirming bug, 2004-07-22-05 trunk Linux.

Regression occurred between 2004-02-27-07 -- 2004-02-28-08
Comment 3 Boris Zbarsky [:bz] (Out June 25-July 6) 2004-07-23 11:00:54 PDT
In that range, the most likely culprit is bug 235778...
Comment 4 Mats Palmgren (:mats) 2004-07-23 11:20:07 PDT
Yes, using nsLineLayout::RelativePositionFrames() from rev. 3.182 works for me.
Comment 5 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2004-07-23 19:54:51 PDT
The problem is this chunk of code in nsLineLayout::TrimTrailingWhitespaceIn:

    if (childSpan) {
      // Maybe the child span has the trailing white-space in it?
      if (TrimTrailingWhiteSpaceIn(childSpan, aDeltaWidth)) {
        nscoord deltaWidth = *aDeltaWidth;
        if (deltaWidth) {
          // Adjust the child spans frame size
          pfd->mBounds.width -= deltaWidth;

Now, just trimming the span frame's bounds does not reduce its overflow area,
because the space remains in its pfd->mCombinedArea. But just trimming
pfd->mCombinedArea is incorrect if the span is relatively positioned with
overflowing absolute children.

To fix this mess, we should just always recompute the overflow area for spans
*from scratch* in RelativePositionFrames, don't bother maintaining it during
frame alignment and so on. I suggest that inline frames should *not* include
their frame bounds or the bounds of their inline children in the mOverflowArea
they return; only inlines with absolute children should accumulate the bounds of
the absolute children in there. So pfd->mCombinedArea will normally be empty
until RelativePositionFrames, which adds in the bounds of the inline frame and
its inline descendants. We will not try to maintain pfd->mCombinedArea during
frame alignment, since that's impossible to do accurately.
Comment 6 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2004-07-23 20:45:58 PDT
Created attachment 154169 [details] [diff] [review]
fix

This does just what I said.

The comments here are a little misleading, because nsTextFrames still report
their overflow area to be exactly their bounds. That doesn't affect the code
but I will tweak the comments to make it clear that can happen.

This patch is a bit aggressive and changes fragile code (but makes the code a
lot less fragile IMHO because we no longer try to maintain mCombinedArea during
alignment). I tested it with many testcases from the relative-positioning bug
tree and also with the IE-compat inline-image-in-table bugs. It seems to work
fine. Probably not 1.7 branch material though.

At some point in the future we want to change things so that the absolute
children of a relatively positioned inline are reflowed *during*
RelativePositionFrames. This will probably completely remove the need to store
mCombinedArea in the pfds, and will also clean up the anomalous treatment of
metrics->mOverflowArea that I'm doing here; inline frames will simply never
return an mOverflowArea from reflow, it will always be ignored.
Comment 7 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2004-08-10 17:04:11 PDT
(In reply to comment #6)
> At some point in the future we want to change things so that the absolute
> children of a relatively positioned inline are reflowed *during*
> RelativePositionFrames. This will probably completely remove the need to store
> mCombinedArea in the pfds, and will also clean up the anomalous treatment of
> metrics->mOverflowArea that I'm doing here; inline frames will simply never
> return an mOverflowArea from reflow, it will always be ignored.

Could you file a bug on this?

(In reply to comment #5)
> Now, just trimming the span frame's bounds does not reduce its overflow area,
> because the space remains in its pfd->mCombinedArea. But just trimming
> pfd->mCombinedArea is incorrect if the span is relatively positioned with
> overflowing absolute children.

I remember marking somebody else's patch review- for something like this.  I'm
wondering if that bug is fixed now.  I suspect not, since I think that bug was
about something else, but I'm not sure.
Comment 8 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2004-08-10 22:07:41 PDT
(In reply to comment #7)
> Could you file a bug on this?

Filed as bug 255139.

> I remember marking somebody else's patch review- for something like this.  I'm
> wondering if that bug is fixed now.  I suspect not, since I think that bug was
> about something else, but I'm not sure.

Neither am I :-)
Comment 9 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2004-08-25 06:06:10 PDT
Checked in.
Comment 10 Mats Palmgren (:mats) 2004-08-29 08:43:07 PDT
*** Bug 257327 has been marked as a duplicate of this bug. ***
Comment 11 Mats Palmgren (:mats) 2004-08-29 18:31:25 PDT
*** Bug 254690 has been marked as a duplicate of this bug. ***
Comment 12 Erik Fabert 2004-12-03 09:09:19 PST
*** Bug 272960 has been marked as a duplicate of this bug. ***
Comment 13 Elmar Ludwig 2005-12-12 03:21:28 PST
*** Bug 319944 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.