Closed
Bug 147558
Opened 23 years ago
Closed 19 years ago
{inc} Moving form elements (button moves when clicked)
Categories
(Core :: Layout: Block and Inline, defect, P3)
Core
Layout: Block and Inline
Tracking
()
RESOLVED
FIXED
Future
People
(Reporter: brian, Unassigned)
References
()
Details
(Keywords: testcase)
Attachments
(6 files, 5 obsolete files)
369 bytes,
text/html
|
Details | |
1.17 KB,
patch
|
Details | Diff | Splinter Review | |
770 bytes,
text/html
|
Details | |
523 bytes,
text/html
|
Details | |
1.21 KB,
patch
|
roc
:
review+
roc
:
superreview+
asa
:
approval1.8b3+
|
Details | Diff | Splinter Review |
3.70 KB,
patch
|
Details | Diff | Splinter Review |
I was browsing around the mozilla.org site, when I got to:
http://tegu.mozilla.org/graph/multiquery.cgi?&testname=startup
When clicking the submit button, it is moved to the line below, and the 'click'
is never performed.
Using RC2.
Reporter | ||
Comment 1•23 years ago
|
||
fixed URL, having duplicate http:// line.
Sorry for spam.
Comment 2•23 years ago
|
||
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.
Testcase.
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
Updated•23 years ago
|
Status: UNCONFIRMED → NEW
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
means.
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.
Comment 8•23 years ago
|
||
Reassigning to karnaze
Assignee: attinasi → karnaze
Priority: -- → P3
Target Milestone: --- → Future
Comment 9•22 years ago
|
||
mass reassign to default owner
Assignee: karnaze → table
Component: Layout → Layout: Tables
QA Contact: amar → madhur
Target Milestone: Future → ---
Updated•22 years ago
|
Target Milestone: --- → Future
Comment 10•21 years ago
|
||
*** Bug 187568 has been marked as a duplicate of this bug. ***
Comment 11•21 years ago
|
||
This also happens on www.funcomeunity.de with latest mozilla from trunk on Linux
OS: Windows 2000 → All
Comment 12•20 years ago
|
||
*** Bug 252884 has been marked as a duplicate of this bug. ***
Updated•20 years ago
|
Comment 13•20 years ago
|
||
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
http://www.mozilla.org/newlayout/doc/block-and-line.html) 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)
Comment 14•20 years ago
|
||
Assignee: nobody → bernd_mozilla
Status: NEW → ASSIGNED
Comment 15•20 years ago
|
||
Comment on attachment 156862 [details] [diff] [review]
patch
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?
Comment 18•20 years ago
|
||
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.
Comment 19•20 years ago
|
||
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)
Comment 20•20 years ago
|
||
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.
nsTableRowFrame::ReflowChildren(
...
rv = ReflowChild(kidFrame, aPresContext, desiredSize, kidReflowState,
x, 0, 0, status);
if (cellToWatch) {
nscoord maxWidth = (NS_UNCONSTRAINEDSIZE == availCellWidth)
? desiredSize.width :
desiredSize.mMaximumWidth;
// save the max element width and max width
if (desiredSize.mComputeMEW) {
cellFrame->SetPass1MaxElementWidth(desiredSize.width,
desiredSize.mMaxElementWidth);
...
// save the max element width and max width
cellFrame->SetMaximumWidth(maxWidth);
}
nsTableRowFrame::IR_TargetIsChild(
...
rv = ReflowChild(aNextFrame, aPresContext, cellMet, kidRS,
cellOrigin.x, 0, 0, aStatus);
...
// cache the max-elem and maximum widths
cellFrame->SetPass1MaxElementWidth(cellMet.width,
cellMet.mMaxElementWidth);
cellFrame->SetMaximumWidth(cellMet.mMaximumWidth);
Comment 21•20 years ago
|
||
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
Comment 22•20 years ago
|
||
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.
Comment 23•20 years ago
|
||
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)
Comment 24•20 years ago
|
||
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)
Comment 25•20 years ago
|
||
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.
Comment 26•20 years ago
|
||
s/the regression is/the regression testing/
see http://www.mozilla.org/newlayout/doc/regression_tests.html
Summary: {incr} Moving form elements (button moves when clicked) → {inc} Moving form elements (button moves when clicked)
Comment 27•20 years ago
|
||
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.
Comment 28•19 years ago
|
||
Comment 29•19 years ago
|
||
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]
patch
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.
Thanks!
Attachment #185958 -
Flags: superreview+
Attachment #185958 -
Flags: review?(roc)
Attachment #185958 -
Flags: review+
Comment 31•19 years ago
|
||
Comment on attachment 185958 [details] [diff] [review]
patch
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?
Comment 32•19 years ago
|
||
I added roc's comments to the attachment 185958 [details] [diff] [review].
Updated•19 years ago
|
Attachment #185958 -
Flags: approval1.8b3? → approval1.8b3+
Comment 33•19 years ago
|
||
roc, is the attachment 186485 [details] [diff] [review] OK? if it is so, could you check in to the trunk?
Comment 34•19 years ago
|
||
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.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•