Note: There are a few cases of duplicates in user autocompletion which are being worked on.

[FIX]On builds beginning with 20031009 vertical scroll bar is "too long" by appr. 4000%

RESOLVED FIXED in mozilla1.6alpha

Status

()

Core
Layout
P1
minor
RESOLVED FIXED
14 years ago
14 years ago

People

(Reporter: amano, Assigned: bz)

Tracking

({regression, testcase})

Trunk
mozilla1.6alpha
x86
All
regression, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

14 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6a) Gecko/20031009 Firebird/0.7+ (aebrahim)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6a) Gecko/20031009 Firebird/0.7+ (aebrahim)

From Mozillazine Thread
http://forums.mozillazine.org/viewtopic.php?t=28590&postdays=0&postorder=asc&start=0
:
debris303 wrote:
on http://www.xsibase.com the menu bar on the left is stretched by ~4000% on the
y-axis. This happens on aebrahims 20031008 SSE build and on the 20031009 Mozilla ni.

I can confirm the bug. I think it's a bug, although the site is not valid html.
but before firebird's quirks mode did handle the code properly.

Reproducible: Always

Steps to Reproduce:
1.Go to site www.xsibase.com
2.Scroll Down
Note that the content ends, but you can scroll down a long empty space

Actual Results:  
Note that the content ends, but you can scroll down a long empty space


Expected Results:  
When the content ends, the vertiacal scrollbar should reach the bottom.
(Assignee)

Comment 1

14 years ago
Hmm.. This regressed as a result of the fix for bug 219693, I bet.  The site has
height:100% tables around that part of the woods.

If someone could create a minimal testcase, that would be much appreciated.
(Assignee)

Updated

14 years ago
Keywords: qawanted
(Assignee)

Updated

14 years ago
Blocks: 222451
No longer blocks: 222451
*** Bug 222451 has been marked as a duplicate of this bug. ***
Created attachment 133472 [details]
Testcase (far from minimal though)

Loading this in 2003-10-16-05 generates the content (see first textarea):
<div id="AWMEL0" ... style="... height: 12618px; ..." ...>

In Mozilla 1.5 I get:
<div ... style="... height: 376px; ..." id="AWMEL0">
OS: Windows XP → All
(Assignee)

Comment 4

14 years ago
Created attachment 133473 [details]
Minimal testcase, I think

The site's menu system does some funky stuff, but the heart of it is that one
table gets too big and the menu system scales everything else accordingly...
(Assignee)

Updated

14 years ago
Attachment #133473 - Attachment is patch: false
Attachment #133473 - Attachment mime type: text/plain → text/html
(Assignee)

Comment 5

14 years ago
Created attachment 133475 [details] [diff] [review]
Proposed patch

This fixes the site, the "far from minimal" testcase, and the minimal-ish
testcase, as well as the URL in bug 222451.

The core issue is that with the change in bug 219693 we now examine the first
two ancestor block/area frames.  This is necessary because in cases when the
body has overflow set both the body and the html are area frames, and we want
to use the height of the html if the body has auto height.

Now a positioned div gets an _area_ frame, not a block frame.  So for a 100%
height table in a positioned div that has no positioned ancestors we used to
look at its area frame and then not look at the area frame of the <html>.  With
the patch in bug 219693 we suddenly set that 100% height table to the height of
the <html> frame.

This patch makes us stop going up the frame tree once we hit a positioned
(absolutely or fixed) ancestor frame.  I think that's a much more intuitive
behavior anyway, since I would not expect kids of a positioned frame to size
based on the containing block of the positioned frame...

I've run the regression tests and they pass (though I suspect that's just a
matter of us having no regression tests testing this.... ;) )
(Assignee)

Comment 6

14 years ago
Taking.
Assignee: other → bzbarsky
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: qawanted → regression
Priority: -- → P1
Summary: On builds beginning with 20031009 vertical scroll bar is "too long" by appr. 4000% → [FIX]On builds beginning with 20031009 vertical scroll bar is "too long" by appr. 4000%
Target Milestone: --- → mozilla1.6alpha
(Assignee)

Comment 7

14 years ago
Comment on attachment 133475 [details] [diff] [review]
Proposed patch

roc, can you review?  Or should I poke dbaron?
Attachment #133475 - Flags: superreview?(roc)
Attachment #133475 - Flags: review?(roc)
This code doesn't match the comment. If the frame is positioned then you always
break and you never use it as the percentage base.

Are you sure you want IsAbsolutelyPositioned() instead of just IsPositioned()?

Also,
> nsCOMPtr<nsIAtom> pseudo = 
Make this a regular ptr
Created attachment 133508 [details]
Testcase #3

The suggested patch breaks this testcase, before: 100px after: auto.
This works for me:

      if (NS_AUTOHEIGHT == rs->mComputedHeight) {
        if (rs->frame->GetStyleDisplay()->IsAbsolutelyPositioned()) {
          break;
        } else {
          continue;
        }
      }
Keywords: testcase
(Assignee)

Updated

14 years ago
Attachment #133475 - Flags: superreview?(roc)
Attachment #133475 - Flags: review?(roc)
(Assignee)

Comment 11

14 years ago
Created attachment 133539 [details] [diff] [review]
Patch that does what it should

Indeed... Mats, thanks for the idea.
Attachment #133475 - Attachment is obsolete: true
(Assignee)

Updated

14 years ago
Attachment #133539 - Flags: superreview?(roc)
Attachment #133539 - Flags: review?(roc)
Attachment #133539 - Flags: superreview?(roc)
Attachment #133539 - Flags: superreview+
Attachment #133539 - Flags: review?(roc)
Attachment #133539 - Flags: review+
(Assignee)

Comment 12

14 years ago
Patch and testcases all checked in.  

roc, I'm not in fact sure I want IsAbsolutelyPositioned() as opposed to
IsPositioned(), but I think that I want layout to not change if someone
relatively positions something without specifying an offset...
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.