Closed Bug 55086 Opened 24 years ago Closed 23 years ago

WRMB: INPUT and TEXTAREA control's value misplaced on reflow

Categories

(Core :: Layout, defect, P3)

Product:
Core
Component:
Layout
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.5

People

(Reporter: jwbaker, Assigned: waterson)

References

Details

(Keywords: topembed, Whiteboard: fixed-on-trunk)

Attachments

(4 files, 1 obsolete file)

Testcase
1.42 KB, text/html
Details
be sure to sync child views after moving the frame
590 bytes, patch
dbaron
: review+
dbaron
: superreview+
dbaron
: approval+
Details | Diff | Splinter Review
more aggressive patch
23.88 KB, patch
Details | Diff | Splinter Review
as above, but check NS_FRAME_NO_MOVE_VIEW before repositioning child views in nsContainerFrame::FinishReflowChild()
23.94 KB, patch
dbaron
: review+
attinasi
: superreview+
asa
: approval+
Details | Diff | Splinter Review 
Linux 2000-10-03-08 M18 build.  If a table reflows due to one row changing to
visibility: collapse, the input's value is drawn outside of the input widget. 
To reproduce:

1) Load the attached test case
2) Type something in the second input box
3) Hit enter

The first table row disappears and the second row moves up.  Unfortunately, the
value that was in the second input box stays put until you do some more typing.

This isn't that serious but it messes up dynamic forms pretty badly.  I'd love
to see this fixed for rtm but I haven't the faintest idea how to fix it.
Attached file Testcase — 

    
  
Keywords: css2

    
    

    
  
Yes, this looks very bad - reassigning
Assignee: rods → beppe

    
    

    
  
This has to be layout, not editor.

Assignee: beppe → buster

    
    

    
  
sorry, at this point we're only looking at critical bugs for NS6 RTM.  This
won't make it, unless someone outside of Netscape proposes a small, safe patch.

OS: Linux → All
Hardware: PC → All
Target Milestone: --- → Future

    
    

    
  
*** Bug 68056 has been marked as a duplicate of this bug. ***

    
    

    
  
This happens on any incremental reflow, see bug 68056.  No need to have dynamic
forms; slow-loading images are enough.  CSS2 is not an issue here.  Nominating;
this looks terrible when it happens.
Keywords: css2 → mozilla0.9, 
          
            nsbeta1

    
    

    
  
updating summary to reflect what's really going on here.

Bug 69294 has a testcase very similar to bug 68056...
Summary: INPUT control's value misplaced when table collapses → INPUT control's value misplaced on reflow

    
    

    
  
*** Bug 69294 has been marked as a duplicate of this bug. ***

    
    

    
  
QA Contact Update
QA Contact: bsharma → vladimire

    
    

    
  
In Mozilla 0.9 (2001050515) Win32, this bug seems to have went away?  The test
cases aren't "broken" anymore.  I am away from my linux boxen for a few days so
I can't try it there........  

    
    

    
  
This may have been partially "fixed" (ie masked) by hyatts non-painting checkin.

The testcase on this bug still does not work since it _just_doesn't_reflow_ when
the row is removed....  That's incorrect and prevents testing of this bug.

Linux build 2001-05-07-08

    
    

    
  
But the testcase doesn't actually work at all.  When the top row disappears, the
bottom row is supposed to shift up and take its place.

    
    

    
  
Yeah, what Boris said.  Is there a bug on reflow not happening?

    
    

    
  
*** Bug 85251 has been marked as a duplicate of this bug. ***

    
    

    
  
Please note that Bug 85251 features a much simpler test case that does not
involve a dynamic form but only a textfield and image in a table.

The test case comes from the http://www.google.com/help/features.html page.


    
    

    
  
*** Bug 85754 has been marked as a duplicate of this bug. ***

    
    

    
  
*** Bug 85692 has been marked as a duplicate of this bug. ***

    
    

    
  
*** Bug 89313 has been marked as a duplicate of this bug. ***
Keywords: mozilla0.9.3
Summary: INPUT control's value misplaced on reflow → INPUT and TEXTAREA control's value misplaced on reflow

    
    

    
  
*** Bug 89363 has been marked as a duplicate of this bug. ***

    
  
Keywords: 4xp

    
    

    
  
*** Bug 90835 has been marked as a duplicate of this bug. ***

    
    

    
  
*** Bug 81873 has been marked as a duplicate of this bug. ***

    
    

    
  
*** Bug 86099 has been marked as a duplicate of this bug. ***

    
    

    
  
Marking mostfreq at 10 dups.
Keywords: mostfreq

    
    

    
  
*** Bug 89335 has been marked as a duplicate of this bug. ***

    
    

    
  
*** Bug 92067 has been marked as a duplicate of this bug. ***

    
    

    
  
*** Bug 92259 has been marked as a duplicate of this bug. ***

    
    

    
  
*** Bug 92273 has been marked as a duplicate of this bug. ***

    
    

    
  
*** Bug 92277 has been marked as a duplicate of this bug. ***

    
    

    
  
I don't think the original description (with test case) is a bug. After step 3,
there should be 2 rows in the table and the text should be in the 2nd row. The
1st row should be collapsed but still there. The only way to make the 1st row
visible again is to remove all of the text in the 2nd row and hit enter. 

This bug is old and it looks like collapsing rows may have regressed. I'm not
seeing the 1st row actually collapse. But that is another bug.

Why are so many other bugs being marked as a dup of this one? This bug involves
collapsing rows on a table.


Assignee: buster → karnaze

    
    

    
  
The original desciption says that:

1)  The first row collapses
2)  The input element moves as a result.
3)  The _text_ that was in the input element does not move with the input element.

The row-collapsing is just a way to make the location of the input element
change.... It is not central to the bug.

The same bug can be reproduced by making input elements move in other ways.  For
example, reflows when an image with no size specified in the HTML is loaded. 
The common symptom in all these is that the text that was in the textbox ends up
sort of floating somewhere outside it.

    
    

    
  
Either that or the blinking cursor that is supposed to be in a selected textbox
is flashing somewhere outside of the box.

Hate to make a comment for this small addition, but someone might find it useful....

    
    

    
  
this looks identical to bug 75080

    
    

    
  
*** Bug 75080 has been marked as a duplicate of this bug. ***

    
  
Blocks: advocacybugs

    
    

    
  
*** Bug 93086 has been marked as a duplicate of this bug. ***

    
    

    
  
*** Bug 93434 has been marked as a duplicate of this bug. ***

    
    

    
  
*** Bug 93473 has been marked as a duplicate of this bug. ***

    
    

    
  
Reassigning to pollmann. 
Assignee: karnaze → pollmann

    
    

    
  
*** Bug 91189 has been marked as a duplicate of this bug. ***

    
  
Status: NEW → ASSIGNED
Keywords: topembed
Target Milestone: Future → mozilla0.9.4

    
    

    
  
*** Bug 93802 has been marked as a duplicate of this bug. ***

    
    

    
  
*** Bug 94677 has been marked as a duplicate of this bug. ***

    
    

    
  
*** Bug 94910 has been marked as a duplicate of this bug. ***

    
    

    
  
Sending out a flare: I've not made much progress on this topembed bug, but as
noted above, it seems to be dependant on an input being inside a table, and
coming after an image with it's size set to something other than it's native
size, like this:

<img src="http://bugzilla.mozilla.org/ant.jpg" height=1><br>
<table border height="100">
 <tr>
  <td>
   <span><center><input type="text" value="text widget value"></center></span>
  </td>
 </tr>
</table>
Every time I see the incorrect positioning, I see the view for the input being
positioned at coordinates other than where the frame is, and this is on top of a
very large reflow stack (inside table reflow).  I'll try to capture a stack of
this happening, but I can't make heads or tails of it, or understand how the
calculations go awry...


    
    

    
  
This sounds like a core layout issue.

    
    

    
  
Doing a best-guess reassign to get coverage on this topembed bug...
Assignee: pollmann → attinasi
Status: ASSIGNED → NEW
Component: HTML Form Controls → Layout

    
    

    
  
Accepting
Status: NEW → ASSIGNED

    
    

    
  
Load sharing.
Assignee: attinasi → waterson
Status: ASSIGNED → NEW

    
    

    
  
what's the current progress on this bug?
Keywords: mostfreq

    
    

    
  
Same as bug 92464?

    
    

    
  
*** Bug 92464 has been marked as a duplicate of this bug. ***

    
    

    
  
adding WRMB and cc'ing myself
Summary: INPUT and TEXTAREA control's value misplaced on reflow → WRMB: INPUT and TEXTAREA control's value misplaced on reflow

    
    

    
  
karnaze: I looked at this tonight and I think that at least one problem is as
follows. With the test case referred to in attachment 47296 [details] (and attachment
46835 [details]), we are ending up with an incremental reflow targeted at the image frame
once the image's width and height information arrives. This will not have any
effect on the available width, so when we arrive at the outer table frame, it
detects that |aOuterRS.availableWidth == mPriorAvailableWidth|, and does the
fast path. Unfortunately, the table frame's y-coordinate will have changed, but
none of the views will have been updated.
Status: NEW → ASSIGNED

    
    

    
  
The above patch forces views to be synchronized, even in the case where no table
reflow is necessary. This accomodates the case where we're simply jiggling the
y-coordinate of the frame. karnaze: what do you think?

    
    

    
  
This also appears to fix attachment 43207 [details] [diff] [review], presumably because we're running into
a similar situation where the table's width hasn't changed but we're altering
its x-coordinate.
Keywords: patch

    
    

    
  
waterson, I think a better patch would be to have the block (e.g. the 
body in this case) reposition views when it moves children (e.g. the table in 
this case). This is more efficient, more consistent with how it works in other 
parts of the system, and more general than just fixing the table cases.

all, I tried the patch on the 1st attachment in this bug, and it did not fix it. 
I think this bug has become a meta bug, with all of the dups that may or may not 
get fixed with this or other patches.

    
    

    
  
karnaze, you were right. We're not synchronizing child views in
nsBlockReflowContext::DoReflowBlock(). Patch anon.

    
    

    
  
[s]r=attinasi

    
    

    
  
Karnaze suggested I ought look for other cases in the block code where this
happens, and it turns out there are one or two. So, I went ahead and put
together a more aggressive fix:

1. Change nsContainerFrame::PositionFrameView() to position child views. 

2. Eliminate the ``aView'' parameter from that method, because the view can be
   retrieved from the frame and need not be done by the caller.

3. Eliminate the NS_FRAME_NO_MOVE_CHILD_VIEWS flag, which was read-only; i.e.,
   never set by anyone.

    
    

    
  


  

    
    

    
  
So, I think what I'd like to do is check in the less aggressive fix (attachment
47736 [details] [diff] [review]) for mozilla-0.9.4, and then land the more aggressive fix when the tree
opens for mozilla-0.9.5. Does that sound reasonable? dbaron/attinasi, could you
take a look at attachment 47810 [details] [diff] [review]?

thanks!

    
    

    
  
Does the change in semantics of NS_FRAME_NO_MOVE_VIEW hurt anything?  (It now
causes no movement of child views, right?)

    
    

    
  
You're probably referring to the changes in nsContainerFrame.cpp to
nsContainerFrame::FinishReflowChild()? Although the patch didn't show it, this
method this method doesn't heed the NS_FRAME_NO_MOVE_VIEW flag (comments
notwithstanding). In fact, since I've removed the NS_FRAME_NO_MOVE_CHILD_VIEWS
flag, it doesn't pay any attention to the aFlags argument at all!

This may surprise some of its consumers, most notably:

- nsComboboxFrame, which uses NS_FRAME_NO_MOVE_VIEW, NS_FRAME_NO_SIZE_VIEW,
  and NS_FRAME_NO_VISIBILITY
- nsFieldsetFrame, which uses NS_FRAME_NO_MOVE_FRAME
- nsBoxToBlockAdaptor, which uses NS_FRAME_NO_MOVE_FRAME

So, although I haven't changed the semantics here, we probably ought to file
another bug to see if we should alter FinishReflowChild() to heed aFlags.

    
    

    
  
Oh wait, I'm sorry. It passes aFlags to SyncFrameViewAfterReflow(), which does
pay attention to the flags. I suppose I ought to continue to condition position
of the child views pased on NS_FRAME_NO_MOVE_VIEW. Let me respin the patch.

    
    

    
  
Hmmm.  I now realize I was looking at nsBox.cpp.

    
    

    
  
Comment on attachment 47736 [details] [diff] [review]
be sure to sync child views after moving the frame

blizzard said by email that the "safe" patch has approval

        Attachment #47736 -
        Flags: superreview+

        Attachment #47736 -
        Flags: review+

        Attachment #47736 -
        Flags: approval+

    
    

    
  
``Safe'' fix landed for mozilla-0.9.4. Moving to mozilla-0.9.5.
Target Milestone: mozilla0.9.4 → mozilla0.9.5

    
    

    
  
please check-in also to 0.9.2. thanks

    
    

    
  
Attachment 47736 [details] [diff] checked in on trunk and mozilla-0.9.2 branch.

    
    

    
  
My changes for bug 86947 exposed a similar problem for listboxes that was fixed
(I think -- it's sometimes hard to reproduce) by making a change almost
identical to attachment 47736 [details] [diff] [review] in nsContainerFrame::ReflowChild.  The "more
aggressive patch" would do the equivalent of that change as well.

    
    

    
  
http://twig.screwdriver.net/demo/ still has the problem, is it going to be fixed
by the aggressive patch, or should a separate bug be filed for this testcase?

    
    

    
  
http://twig.screwdriver.net/demo/ appears fine in my tree, AFAICT, so I'm going
to say ``yes, this will be fixed''.

    
    

    
  
patch 47813 looks great - [s]r=attinasi

    
    

    
  
Comment on attachment 47813 [details] [diff] [review]
as above, but check NS_FRAME_NO_MOVE_VIEW before repositioning child views in nsContainerFrame::FinishReflowChild()

sr=attinasi

        Attachment #47813 -
        Flags: superreview+

    
    

    
  
*** Bug 98297 has been marked as a duplicate of this bug. ***

    
    

    
  
Comment on attachment 47813 [details] [diff] [review]
as above, but check NS_FRAME_NO_MOVE_VIEW before repositioning child views in nsContainerFrame::FinishReflowChild()

r=dbaron

        Attachment #47813 -
        Flags: review+

    
    

    
  
Checked in attachment 47813 [details] [diff] [review] onto the trunk.
Whiteboard: fixed-on-trunk

    
    

    
  
*** Bug 90427 has been marked as a duplicate of this bug. ***

    
    

    
  
could we check this into 0.9.2 branch?

    
    

    
  
Comment on attachment 47813 [details] [diff] [review]
as above, but check NS_FRAME_NO_MOVE_VIEW before repositioning child views in nsContainerFrame::FinishReflowChild()

a=asa for checkin to the 0.9.4 branch.

        Attachment #47813 -
        Flags: approval+

    
    

    
  
Attachment 47813 [details] [diff] checked in on mozilla-0.9.4 branch.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED

    
    

    
  
verifying build 2001-09-10-06-0.9.4 windows 98
Status: RESOLVED → VERIFIED

    
    

    
  
I just realized that this fix sucks. We typically call PositionFrameView()
before reflowing a frame to position the view correctly. I've altered
PositionFrameView() in such a way that it will walk the entire frame tree if the
frame doesn't have a view.

I'll file another bug to fix this.

    
    

    
  
Filed bug 99722.

    
    

    
  
I may have found a regression:

http://isomorphisms.addr.com/archives/00000130.html#comments

If you click in the topmost text field the caret appears too high (plus the
autofill of my email address is too high)

This site was working fine with a build from a few days ago.

Build id 2001101021

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: