Closed Bug 147558 Opened 22 years ago Closed 19 years ago

{inc} Moving form elements (button moves when clicked)


(Core :: Layout: Block and Inline, defect, P3)






(Reporter: brian, Unassigned)




(Keywords: testcase)


(6 files, 5 obsolete files)

I was browsing around the site, when I got to:
When clicking the submit button, it is moved to the line below, and the 'click'
is never performed.

Using RC2.
fixed URL, having duplicate http:// line.
Sorry for spam.
Bug is same in RC3 on Linux.  This doesn't seem to go away by fixing simple html
syntax errors (such as missing body, head, etc, however, the Submit button is
incorrect code.  <input type="submit" value="submit"> is the Correct submit code.

<input TYPE="button" VALUE="Submit" onClick="scrape_for_names()"> is the code
used in this document to submit.  Notice the incorrect type.  Fixing the type to
submit doesn't fix the "skip" bug though.
Attached file testcase (obsolete) —

Please set status whiteboard to testcase.

Note: Placing <form> inside <ul> is not valid HTML 4 but I'm guessing this is
still a bug.
Attachment #85298 - Attachment is obsolete: true
Attached file testcase
Improved (correct) testcase.
Keywords: testcase
Ever confirmed: true
The reason the "click is never performed" is that the form button has moved so
that the mouse pointer is no longer over it when the button is released.  To
observe this:  press down the mouse over the form submit button (but don't let
go); then if the submit button jumps around, move the mouse pointer back over it
before releasing the mouse button.

Workarounds I have found:
1) Refrain from nesting forms in a list nested in a table.
2) Specify a width for any of the elements enclosing the form (for example,
<table width=250>, or <li style="width: 250px">, etc.).
3) Include enough text somewhere in the li/ul to cause line-wrap.
4) Add onMouseDown="return false;" to the input element.  This is the most
interesting one, because onMouseDown="return true;" does not work as a
workaround, suggesting that the layout shake-up happens after the onMouseDown
event falls through per usual.  But it does work when we interrupt things with
an alert box -- onMouseDown="alert(''); return true;".  Not sure what all this

I don't think this bug is really about "moving form elements" (form elements
jumping around), or about missing click events.  The test case makes it easy to
narrow down the problem to a form nested in a table nested in a list. 
Furthermore, it is clear to me that the bug is not that things move around
arbitrarily, but rather that there is some change in the width of the form's
enclosing elements resulting in a reflow that makes it appear as though there
are "moving form elements."  Note that the button isn't always the thing to jump
around; it depends on where you position extra text within the ul/li that
encloses the form (for example, try putting text after the button within the
enclosing li or ul).  I would summarize what I observe this way:  "form width
shrinks on mousedown for input type=button|submit|reset|file when form is nested
in list nested in table."
I observe this behavior on daily build 2002052508, Win98.  I hope all this
helps.  Perhaps someone has something to add to these observations.
Changing QA Contact
QA Contact: petersen → amar
Reassigning to karnaze
Assignee: attinasi → karnaze
Priority: -- → P3
Target Milestone: --- → Future
mass reassign to default owner
Assignee: karnaze → table
Component: Layout → Layout: Tables
QA Contact: amar → madhur
Target Milestone: Future → ---
Target Milestone: --- → Future
*** Bug 187568 has been marked as a duplicate of this bug. ***
This also happens on with latest mozilla from trunk on Linux
OS: Windows 2000 → All
*** Bug 252884 has been marked as a duplicate of this bug. ***
Blocks: 195770, 237159
Summary: Moving form elements → Moving form elements (button moves when clicked)
         button 024EC9A0 d=756,264 me=756
         text 024ECC7C r=2 a=0,UC c=UC,UC cnt=927
         text 024ECC7C d=0,0 me=0
        block 024EC72C d=1656,264 me=756 m=1656
       block 024EC594 d=2136,264 me=1236 m=1656    <<<<<<<<<<<<<<<<<<
      block 024EC10C d=2136,456 me=1236 m=1656
     cell 024EC0AC d=2160,480 me=1260 m=1680

This is a block reflow issue. The block does not update correctly the
mMaximumWidth ( The maximum width that the frame would consume if it were
reflowed with an unconstrained available width. ref. Waterson during incr. reflow.
I don't believe that the mMaximumWidth could ever be smaller than line->Xmost in
an unconstrained block.
Assignee: core.layout.tables → nobody
Component: Layout: Tables → Layout: Block and Inline
QA Contact: madhur → core.layout.block-and-inline
Summary: Moving form elements (button moves when clicked) → {incr} Moving form elements (button moves when clicked)
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → bernd_mozilla
Comment on attachment 156862 [details] [diff] [review]

the patch has passed the rtest, so doing the the correct thing should be fine.
Attachment #156862 - Flags: superreview?
Attachment #156862 - Flags: review?
Attachment #156862 - Flags: superreview?(dbaron)
Attachment #156862 - Flags: superreview?
Attachment #156862 - Flags: review?(dbaron)
Attachment #156862 - Flags: review?
I need to check what coordinate space nsLineBox::mBounds is in. 
nsBlockFrame::ReflowLine and PostPlaceLine both do:

    aLine->mMaximumWidth = aLine->mBounds.XMost();

I think that needs to subtract the left border and padding, but I'm not sure.
In other words, does this patch cause a regression on attachment 85310 [details] with the
form element removed?
Yes, it does cause a regression. Not a visible one without borders, but it
reports a too large mMaxWidth (it is just by borderpadding.left too large)
during the incr. reflow, this will give a too large table, which does not
tightly wrap the input.
Attached patch another attemptSplinter Review
Somewhere the offset needs to be added. I can not claim that I know block
reflow good enough to know where this place is. This place now fixes the
testcase and does not regress as the other patch the scenario that David
described. Both patches have passed the rtest (so far for the meaning of rtests
:-() I will stop this buisiness now, somewhere in the maximumWidth computation
is a bug, that the xOffset is not taken into account.
Attachment #156862 - Attachment is obsolete: true
Attachment #156862 - Flags: superreview?(dbaron)
Attachment #156862 - Flags: review?(dbaron)
Assignee: bernd_mozilla → nobody
No longer blocks: 237159
Attached patch other patch (obsolete) — Splinter Review
Please refer to the following two functions, nsTableRowFrame::ReflowChildren
and nsTableRowFrame::IR_TargetIsChild. With the initial reflow, maxWidth is set
to the MaximumWidth of the cellFrame. I think that with the incremental reflow,
cellMet.width should be set.

	  rv = ReflowChild(kidFrame, aPresContext, desiredSize, kidReflowState,

			   x, 0, 0, status);

	  if (cellToWatch) { 
	    nscoord maxWidth = (NS_UNCONSTRAINEDSIZE == availCellWidth) 
				? desiredSize.width :
	    // save the max element width and max width
	    if (desiredSize.mComputeMEW) {
	    // save the max element width and max width

    rv = ReflowChild(aNextFrame, aPresContext, cellMet, kidRS,
		     cellOrigin.x, 0, 0, aStatus);
    // cache the max-elem and maximum widths
Comment on attachment 165708 [details] [diff] [review]
other patch

Sorry, this patch is wrong since a table cell spreads unexpectedly.
Attachment #165708 - Attachment is obsolete: true
Attached patch other patch (obsolete) — Splinter Review
I think that a width of the indent is added to both mMaxElementWidth and
mMaximumWidth, if the value of mSpace.x means a width of the indent.
Blocks: 189367
Attached patch patch (obsolete) — Splinter Review
In the initial reflow, it is not requested to compute the maximum width.
In the incremental reflow, the computed maximum width does not include the
x-position rect in the space manager. I think that the maximum x-most of each
line should be computed.
Attachment #165805 - Attachment is obsolete: true
Attachment #182370 - Flags: review?(bernd_mozilla)
I should not review this, see comment #19. But before you ask roc, please
describe what sort of testing this patch got. The regression is a required
minimum but not sufficient to declare the patch correct.
Attachment #182370 - Flags: review?(bernd_mozilla)
Attached file testcase2
I tested using the test case. The list item's margin in the second table has
percentage value. The testing is a click and text zoom. I checked that there
was no change in table size.
s/the regression is/the regression testing/

Summary: {incr} Moving form elements (button moves when clicked) → {inc} Moving form elements (button moves when clicked)
See Bug 269643 - When clicking link, page redraws with different layout, click
is ignored  

This shows a very similar problem but no form, button or list are involved. 
Just nested tables.
Blocks: 291759
Attached patch patchSplinter Review
I think that influence is smaller than previous patch, although the value of
availSpace.x is set to mSpace.x.
Attachment #182370 - Attachment is obsolete: true
Attachment #185958 - Flags: review?(roc)
Comment on attachment 185958 [details] [diff] [review]

Looks good.

Please add a comment here mentioning that line->mMaximumWidth includes the left
border/padding of this block, but not the right.

At nsBlockFrame.cpp line 1457 or thereabouts, dbaron wrote "	  // XXXldb Why
right and not left?" ... Replace that with a comment mentioning that the reflow
state mMaximumWidth includes the left border/padding but not the right.

Put similar comments where those variable are declared in nsBlockReflowState.h
and nsLineBox.h.

Attachment #185958 - Flags: superreview+
Attachment #185958 - Flags: review?(roc)
Attachment #185958 - Flags: review+
Comment on attachment 185958 [details] [diff] [review]

I think that this patch is low-risk. To prevet the line from breaking or the
elements from moving, the left border/padding of the nested blocks is added to
the maximum width in the line box.
Attachment #185958 - Flags: approval1.8b3?
I added roc's comments to the attachment 185958 [details] [diff] [review].
Attachment #185958 - Flags: approval1.8b3? → approval1.8b3+
roc, is the attachment 186485 [details] [diff] [review] OK? if it is so, could you check in to the trunk?
masayuki, attachment 185958 [details] [diff] [review] was approved for 1.8b3. However, I ask to check in
attachment 186485 [details] [diff] [review]. Could you check in to the trunk? and please check Bug 291759.
Hardware: PC → All
checked in. Thanks Hideo, the update is great.
Closed: 19 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.