Closed Bug 119849 Opened 23 years ago Closed 22 years ago

{inc}Forms move around when entering text and pressing submit and headings often won't center on page with fixed positioning.

Categories

(Core :: Layout, defect, P3)

Product:
Core
Component:
Layout
Platform:
x86
Windows 98
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.0

People

(Reporter: ronbu, Assigned: john)

References

Details

(Keywords: testcase, topembed+, Whiteboard: [bae:20020131][adt2][FIX][FIXED_ON_TRUNK])

Attachments

(5 files, 1 obsolete file)

Testcase. Button in frame scrolling=no Click here.
416 bytes, text/html
Details
Minimized Testcase
234 bytes, text/html
Details
Dead Simple testcase with body overflow: hidden
120 bytes, text/html
Details
Reflow Debugging
5.88 KB, text/plain
Details
Patch
2.49 KB, patch
hewitt
: review+
attinasi
: superreview+
jesup
: approval+
Details | Diff | Splinter Review 
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.7+) Gecko/20020111
BuildID:    2002011103

In the test case to be entered, the headings in a absolute positioned
DIV below a fixed positioned DIV often will not center even though
they are set by a stylesheet to text-align:center .  The form will be
aligned to the left, if the width of the window is wide enough for the 
heading on the line above to fit on one line, even though the form contains a
table which is set to margin-left:auto; margin-right:auto .  The form is wrapped
in a DIV which is set to text-align:center .
The form is aligned to the center, if the heading on the line above is displayed
on two lines.  Once text is entered in a form which starts at the left, the form
immediately shifts to the center.  When the submit buttom is pushed, the form
shifts to the left.

Reproducible: Always
Steps to Reproduce:
1.  Go to the attached page.
2.  Enter text in the form.
3.  Press submit.

Actual Results:  The form shifts to the left.

Expected Results:  The form should remain in the center.
Keywords: testcase
I am also seeing this behavior on the following site:
https://www.retirementpassport.com:443/rss/servlet/ServiceDispatch/0304

1. Go to the above URL
2. Click on the "Enter" image
3. Fill in the fields
4. Click Submit

The form then moves around. 

I am using build 2002011503 on Windows 2K
I am still seeing this behavior in the 2002012903 build on Win2K. Any
possibility that someone will be looking in to this bug? 
->Layout.  Looks like some sort of incremental reflow issue, except it might be
the initial reflow that's broken.
Assignee: dbaron → attinasi
Component: Style System → Layout
QA Contact: ian → petersen
Summary: Forms move around when entering text and pressing submit and headings often won't center on page with fixed positioning. → {inc}Forms move around when entering text and pressing submit and headings often won't center on page with fixed positioning.
Confirming issue in the Jan 30th build.
Status: UNCONFIRMED → NEW
Ever confirmed: true
I saw the shift only on the very first try, if I reload the page and try again, 
the shift does not occur. Setting to moz1.0, P3
Priority: -- → P3
Whiteboard: [bae:20020131]
Target Milestone: --- → mozilla1.0
For the test case attached, reloading the page does in fact fix the problem.
However, for the link I provided in Comment #2, reloading does not make the
problem stop. Plus, reloading that page on retirementpassport will cause login
problems.

Also, I'm not sure if this is related or not (I couldn't find another bug listed
for this problem), but sometimes when I click on a link in a page the page will
"jump" similiar to the way that the forms rearrange themselves. This is quite
annoying since it means I have to click twice as many times (and move the mouse
around) to perform and action that should only require one click.
Marking nsbeta1+
Keywords: nsbeta1+
*** Bug 122966 has been marked as a duplicate of this bug. ***
Continuing with Sean's statement in comment #7, if you go to the link that I 
provided in bug 122966 (which has been duped to this one), reloading does not 
solve the problem there either.  That link was

   http://home.pacbell.net/spmorse/census/

and the problem occurs when you click on either of the two large buttons near 
the top of the page.
nominating for topembed.
Keywords: topembed
*** Bug 116367 has been marked as a duplicate of this bug. ***
Marking topembed+
Keywords: topembed → topembed+
*** Bug 128126 has been marked as a duplicate of this bug. ***
*** Bug 120940 has been marked as a duplicate of this bug. ***
This is scrolling=no, and mine.
Assignee: attinasi → jkeiser
Blocks: 124431
I say it's scrolling=no because it exhibits the exact same behavior as bug
124431, which *is* scrolling=no.  A more reduced testcase would be nice, and
might shed light on that whole mess.
bug 120940, resolved duplicate, has a minimal testcase.
That testcase is totally different, in fact.  scrolling=no does not show up in
this testcase, which is why a reduced testcase for *this* bug is still necessary
(and would in fact be wonderful).
How about the testcase in bug 122966?
When you view the source you see "scrolling=no" in both frames.
Gotcha, but the other testcase on this bug doesn't have scrolling=no.  Oh well,
when I get to this bug I'll reduce it if no one else has.
*** Bug 130290 has been marked as a duplicate of this bug. ***
adt2 per adt triage
Whiteboard: [bae:20020131] → [bae:20020131][adt2]
Attached file Minimized Testcase — 
Minimized version of attachment 64782 [details].	Change the table to div or span and it
works again.  Remove either style and it works as expected.
Attachment #64782 - Attachment is obsolete: true
*** Bug 133034 has been marked as a duplicate of this bug. ***
This is a dead simple testcase that shows the button doing its bizarre dance
with a <body overflow=hidden>, which comes from the same root cause: an
nsScrollPortFrame without another scroll-child under it.
Attached file Reflow Debugging — 
The annotated and cleaned-up results of a reflow debugging session with this
testcase.
OK, here's the problem with this: the button jumps to the left because the
<CENTER> is not told how much space it has; therefore the CENTER just takes up
as much space as the button, and no centering will take place at all.

The reason the CENTER takes up just as much space as the button is:

1. nsScrollPortFrame::Reflow() (actually nsBoxFrame::Reflow()) is called.
2. This does the dandy dance and reflows everything correctly.
3. *Then* it calls nsScrollBoxFrame::GetAscent() (for what reason I cannot
remember).
4. This calls child frame -> GetAscent() (nsBoxToBlockAdapter::GetAscent()).
5. This calls nsBoxToBlockAdapter::RefreshSizeCache().  Bingo, GetPrefSize(),
unconstrained reflow.  We are officially reflowing our children with
unconstrained size, my friend!

GetPrefSize() has caused problems in other instances as well, and is probably
the source of most (if not all) of the scrolling="no" problems.  We shouldn't be
doing an actual reflow just to get the preferred size of a friggin' frame! 
There needs to either be another reflow type and everyone obey it, or we need to
reflow again properly, or perhaps we can GetAscent() *before* we reflow the
child frames.  Or trigger the child cache update before then (haaaack).

Perhaps another solution in the short term is to make nsBoxToBlockAdaptor not
calculate every single kind of size at the same time and to not make nsBoxFrame
ask for preferred size when it doesn't need to.
Attached patch Patch — — Splinter Review 
This fixes this problem.  I will now test to ensure it fixes the other
scrolling=no problems (I know of another scrolling=no problem that it probably
won't fix).
The basic problem is, nsBoxToBlockAdaptor wants to reflow unconstrained to get
the ascent when all it has to do is use the result of the most recent reflow. 
GetAscent() seems to me really refer to the ascent that the block either *is*
using or wants to use shortly.  So the result of the most recent reflow should
be sufficient.

This patch uses the result of a reflow (if one is available) to populate
mAscent, so that RefreshSizeCache() and its nightmare reflow is not triggered. 
This may actually improve pageload performance as well.

This also fixes a problem where descent was not getting set correctly.  descent
+ ascent = height is the equation.

We really need to fix nsBoxToBlockAdaptor or reflow at some point.
Blocks: 115046
Blocks: 117433
Blocks: 123253
Blocks: 124090
*** Bug 125508 has been marked as a duplicate of this bug. ***
Blocks: 127514
this might also fix bug 135187
Blocks: 128070
*** Bug 132694 has been marked as a duplicate of this bug. ***
Blocks: 134833
This sounds like it may well fix bug 132156.  Does it?
I went to look but a major style bug (bug 135895) is preventing at least some of
the investigation.  It is fairly likely that this fixes those too, though.
Blocks: 132156
This patch makes sense to me, although I'm not an expert on the box code. 
cc:ing bryner and jag in case they have any better ideas.
Whiteboard: [bae:20020131][adt2] → [bae:20020131][adt2][FIX]
cc'ing waterson, hyatt.
As I understand it, there are no experts on the box code except perhaps hyatt or
hewitt ... hyatt is on vacation for two weeks, and I emailed hewitt today,
hopefully he will be able to r=.
Looks ok to me also.  How much testing has this had?

*** Bug 135187 has been marked as a duplicate of this bug. ***
Testing:
1. This has been tested on Windows XP and Mandrake Linux 8.2.
2. I have run the layout regression tests with no regressions.
3. I have browsed with this over a number of regular sites and they work fine.
4. I tested it specifically against the strange testcases in the 20 bugs
attached to bug 124431 and it worked.

So, r=, anyone?
John,
It looks something big is going on and I can't wait for the build with your patch.
Meanwhile you might want to have a look at bug 24684 and see whether your patch
has an effect on it or whether you have any idea how to attack it.
Comment on attachment 77981 [details] [diff] [review]
Patch

sr=attinasi
Attachment #77981 - Flags: superreview+
Bug 24684 doesn't look like it's related to this bug.
Comment on attachment 77981 [details] [diff] [review]
Patch

makes sense, r=hewitt
Attachment #77981 - Flags: review+
Blocks: 131802
Fix is in the trunk.  Now we ask for adt and drivers.
Keywords: adt1.0.0
Whiteboard: [bae:20020131][adt2][FIX] → [bae:20020131][adt2][FIX][FIXED_ON_TRUNK]
Now that this is on the trunk, pls make sure we get some testing on it, asap.
smoketests are going today, hopefully that's good enough :)
petersen: Could you verify the fix on the trunk? Thanks!
Verified in the OS X trunk (2002-04-12-03) and Windows ME trunk (2002-04-12-03)
builds.
On a few complex multi-level frames sites that suffered badly from the effects
of this, all looks perfectly ok to me. Congratulations.
Blocks: 135608
*** Bug 134833 has been marked as a duplicate of this bug. ***
adt1.0.0+ (on ADT's behalf) for checking into the 1.0 branch. Pls check this one
in asap, and replace adt1.0.0+ with fixed1.0.0 keyword. After QA has verified
the fix is in the branch, pls replace fixed1.0.0, with verified1.0.0.
Keywords: adt1.0.0 → adt1.0.0+
checked in on trunk.
Status: NEW → RESOLVED
Closed: 22 years ago
Keywords: fixed1.0.0
Resolution: --- → FIXED
Marking verified on the April 15th build (2002-04-15-03)under Windows ME.
Status: RESOLVED → VERIFIED
Verified on branch OS X (2002-04-18-05) and Windows ME (2002-04-17-11).
Keywords: fixed1.0.0 → verified1.0.0
*** Bug 121217 has been marked as a duplicate of this bug. ***
Keywords: fixed1.0.0
Blocks: 148373
-  aDesiredSize.descent = 0;
+  aDesiredSize.descent = r.height - ascent;

shouldn't we apply that to nsLeafBoxFrame too ?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: