192267 - choosing SELECT option causes DIV to change position until Reload

Status: RESOLVED FIXED

(Core :: Layout: Positioned, defect)

Description

[Scott J. Miles]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.3b) Gecko/20030207
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.3b) Gecko/20030207

If a SELECT is absolutely positioned inside of a relative DIV, inside a TD,
choosing an option from the SELECT control causes the DIV to reposition itself
incorrectly. Hitting Reload restores the DIV position. This error only occurs if
the TD is at least the second TD in it's TR.

Reproducible: Always

Steps to Reproduce:
1. Create a table with a single TR and two TDs
2. Put a relative DIV inside the second TD with styled width, height = 100%.
3. Put an absolutely positioned SELECT inside the DIV.
4. In the browser, select an option from the SELECT.

Actual Results:  
Selecting an option from the SELECT causes the DIV to change position (appears
to become relative to it's former position).

Expected Results:  
I expect elements to retain their positions when choosing SELECT options.

Comment 1

[Scott J. Miles]

Attachment: Mozilla Jumping Select Example

This test case is almost minimal. The   and the DIV background-color are
there to improve the display, but have no effect on the operation.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: minimal testcase

All the things in that testcase are required except the presence of two tables.
 The idea there is to show that without the   the layout is at least
_consistent_.

Comment 3

[Christopher Hoess (gone)]

Yipes. Over to positioning.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Are you sure this is not tables?

Comment 5

[Christopher Hoess (gone)]

Attachment: Further permutations

Comment 6

[Christopher Hoess (gone)]

The extended testcase shows that there are two closely related problems at work
here. First, absolutely positioned elements should not (AIUI) influence the
width of their containing boxes, as they have been removed from normal flow.
However, table cells containing a mixture of normal (relatively positioned)
content and absolutely positioned content apparently miscalculate their width.
Second, changing an <option> in a <select> presumably triggers a re-calculation
of the <td> width, which now results in a width that is too small, rather than
too large.  I presume that were it possible to trigger a reflow on other types
of absolutely positioned content, they would also yield an incorrect result
different from the initial reflow.

Since this involves <td> width calculation, and the location of the absolutely
positioned content would be correct were those calculations correct, I think
your advice is correct and I will send this to Tables.

Comment 7

[Christopher Hoess (gone)]

(yes, really ->Tables)

Comment 8

[Bernd]

Attachment: reflow log for the second testcase

area 02654410 d=1965,300 me=deadbeef m=0 o=(0,0) 1965 x 330
nsBlockReflowContext: Area(div)(0)@02654410 didn't set max-element-size!

there might be a table problem but a 0xdeadbeef MEW is a positioning problem
and nothing else. In general it might be a good idea to look first at the
reflow log before claiming that it is a table reflow bug

Comment 9

[Bernd]

giving it back to positioning to solve the 0xdeadbeef MEW first.

Comment 10

[Bernd]

Attachment: hack to demonstrate that it is indeed a block problem

With the attached hack the selects stop to move. While one would expect that a
0xdeadbeef mew blows the layout in a debug build completely away, this will not
happen as 0xdeadbeef is negative and will be removed during the reflow at
http://lxr.mozilla.org/seamonkey/source/layout/html/base/src/nsBlockReflowState.cpp#708


standard disclaimer: I dont know block reflow good enough to verify that the
hack is correct, so its more a demonstration where things go wrong

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Why is the table code setting mComputeMEW to true in the pass where it's using
the reflow path?  Shouldn't (especially since the reflow nodes get destroyed
when we follow them, IIRC) the path be used on the final pass?

(I think this just demonstrates another major flaw in our current system of
using the same |Reflow| method for min/pref width computation.)

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(Or is it sending eReflowReason_Incremental with no path at all, which would
also cause this?)

Comment 13

[Bernd]

The table code needs to know the MEW for every table cell. The MEW can change
during an incremental reflow targeted at a frame below the cell, furthermore its
is also likely that the MEW changes during an incr reflow with a style change
type or a dirty reflow. Please note that the table cell MEW plays a twofold roll
in auto layout, a) the cell can not shrink below the MEW, b) the remaining space
between auto cells is redistributed based on the MEW. In order to get the things
correctly the table row requests in auto mode for every incr. reflow where the
row is not the target a MEW from the corresponding cells (
http://lxr.mozilla.org/seamonkey/source/layout/html/table/src/nsTableRowFrame.cpp#1267
).
If during the incr. reflow the  the cell has changed  the necessary reaction to
this is determined at nsTableFrame::CellChangedWidth (
http://lxr.mozilla.org/seamonkey/source/layout/html/table/src/nsTableFrame.cpp#3703
)

Based on the severity of the change  the layout strategy remains unchanged,
rebalanced or reinitialized. Changing the strategy also influences the
NeedReflow flag. 

In order  to get the info about the changed column information down to every
cell that needs to know it, the table tries to issues an additional resize
reflow (nextReason = eReflowReason_Resize; at
http://lxr.mozilla.org/seamonkey/source/layout/html/table/src/nsTableFrame.cpp#1995
If there is really a need for a reflow then at
http://lxr.mozilla.org/seamonkey/source/layout/html/table/src/nsTableFrame.cpp#2020
NeedsReflow will return true and the resize reflow brings the table in
accordance with the new layout strategy. 

Thats my understanding how it works, so for me it looks perfectly valid to
request during an incr. reflow the table cell MEW.

Comment 14

[Mats Palmgren (inactive)]

The patch can be checked in now?

Comment 15

[Bernd]

patch checked in