Closed Bug 91423 Opened 23 years ago Closed 23 years ago

Every other typed char fails to render in a textfield wrapped in an inline style

Categories

(Core :: Layout, defect, P2)

Product:
Core
Component:
Layout
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.8

People

(Reporter: amyy, Assigned: waterson)

References

(
URL
)

Details

(Keywords: testcase, topembed, Whiteboard: after this is fixed verify URL works in bugscape 10622)

Attachments

(6 files, 3 obsolete files)

Screen shot of shifted search text field.
115.28 KB, image/jpeg
Details
Minimal test case.
92 bytes, text/html
Details
minimal test with constrained <td> width to force-wrap
105 bytes, text/html
Details
Source code for Priceline car rental "name your price" page
20.04 KB, text/plain
Details
feh. let's keep it simple.
4.59 KB, patch
dbaron
: review+
attinasi
: superreview+
Details | Diff | Splinter Review
updated patch vs. 0.9.4 branch; incorporates fix for 116230
10.14 KB, patch
rbs
: review+
attinasi
: superreview+
Details | Diff | Splinter Review 
Build: 07-18 branch on WinME-Ja, Win2k-CN, WinXP-Ja and LinuxRH6.2-Ja.

Not reproduce on: 
1. 07-18 branch build on Win2k-En and Win98-En
2. N6.1PR1 Windows/Win2k-CN, Mac 07-18 branch build / Mac9.0-Ja.

Steps to reproduce:
1. Launch browser, and go http://www.msn.com
2. Move the mouse cursor to the search field, start typing - doesn't matter
   English or Japanese/Chinese characters.

Result (see the screen shot later):
1. The search text field which beside the string "SEARCH the web" has been 
shift to next to it.
2. When you start to typing, after 2 or 3 letters, you won't see the new letter
appears, and when you hit "Backspace" key to delete it, the mouse cursor moved
to the before the first letter!  But if you continue hit "Backspace" key, then 
will delelt the last letter, and the cursor will back to the end of last letter.

    
  
Keywords: intl
QA Contact: andreasb → ylong

    
    

    
  
I saw the screen shot and tried it on my machine but I am not sure what is the
problem.

    
    

    
  
As I said before, if you are running on US system, you probably can not see the 
problem.

The search text field original position is next to the string "SEARCH the web", 
and now it's under the string.  Also, when you start typing, some letter won't 
show inside the text area, and has wrong cursor focus problem.

    
    

    
  
Reassign to yokoyama.
ylong, could you try 4.x?
Assignee: nhotta → yokoyama
Summary: Typing error in search field of msn homepage → Typing in search field of msn homepage causes reflow and typing problem

    
    

    
  
I believe it's to do with calculating height/width of text.
The page (www.msn.com) renders correctly until you start 
to type some text. 

assigning to shanjian, cc rbs@maths.uq.edu.au.

Assignee: yokoyama → shanjian

    
    

    
  
Not reproduce on N4.7.

    
    

    
  
Cannot reproduce even when setting 'useAFunctions = 1' in nsGfxFactoryWin.cpp.
I am on Win2K but by forcibly setting this flag, I am forcing Mozilla to use the
'A' functions as if I was on Win95-J, i.e., Mozilla behaves as if it was running
on Win95-J. So only the binary OS calls are different. But even with that, I
cannot reproduce.

However, there are some known weird problems with typing in a <input> textfield,
e.g., bug 79435.

Adding the 'testcase' keyword to note that a minimal testcase is needed.
Keywords: testcase

    
    

    
  
this seems related to layout and ender. Reassign to ender group to look at it. 
beppe- can you find someone in your group to look at this one?
Assignee: shanjian → beppe

    
    

    
  
-->kin

Assignee: beppe → kin

    
    

    
  

      
      
      
      

        Attached file
          
        Minimal test case.
        — 
      

    


  

    
    

    
  
I just a test case which is basically:

<html>
<body>
<table border><tr><td><b>a<input></b></td></tr></table>
</body>
</html>

It seems to be a table and inline style interaction/reflow problem. If I remove
either the table, or the inline style, I can't repro the bug.
Assignee: kin → karnaze

    
    

    
  
I'll take a look at this. I suspect either bustage due to the ``reflow reason''
or getting fudged by a box in a table cell (a la bug 73348).
Assignee: karnaze → waterson
Priority: -- → P2
Target Milestone: --- → mozilla0.9.5

    
    

    
  
Well, this is odd. The minimal test case appears to work fine on Unix. It's
interesting to note that Unix renders the `a' character on the same line as the
input, while windows places the input on the next line (which seems wrong).
Status: NEW → ASSIGNED

    
    

    
  
Mac looks fine, too. This appears to be a win32-specific problem.
OS: All → Windows 2000
Hardware: All → PC

    
    

    
  
If I recall correctly, the bug only seemed to happen if the input field wrapped 
to the 2nd line. Try adding more chars after the 'a' to force it to wrap on 
Unix/Mac.

    
  
OS: Windows 2000 → All
Hardware: PC → All

    
  
Depends on: 73348

    
    

    
  
Zany. The problem here is that we're targeting an incremental reflow at the
continuing frame for the <b>. The continuation gets destroyed when the table
cell demands an unconstrained reflow to recompute the block's max-width. Since
the incremental reflow never makes it to the input element's anonymous <div>
frame, its content doesn't update. Then, when you type the _next_ keystroke, the
<div> frame coalesces the ReflowDirtyChild and delegates to its parent frame
(the <td>) which reflows everybody and makes everything work out.

- The table cell must demand recomputation of the max-width, because the
  incremental reflow could change that.

- The block must dirty the line prior to the <input>, because cases could
  exist where an incremental reflow sent to a continuation could lead to
  frames being pulled back to the prior line.

- The box frame for the nsGfxTextControlFrame2 can't detect any dirty children,
  so it won't reflow the <div> as a side effect of the frame jiggling that the
  block frame is doing.. The frame model looks like:

  Inline(b)<
    Text<
      "a"
    >
  >
  Inline(b)<
    nsGfxControlFrame2<
      GfxScroll(div)<        ScrollPortFrame(div)<
          Area(div)<
            ...
          >
        >
      >
    >
  >

  The Area frame has been marked dirty, but as far as the nsGfxControlFrame2 is
  concerned, it has no dirty children.

Although it seems like the right way to fix this will be to try to get the
incrmental reflow dispatched all the way down to the <div> that requested it, it
seems like it might be more practical to beat on the text control frame until it
realizes that the dirty Area frame ``counts'' as a dirty child.

OTOH, doing that may lead to other bizarre cases (like bug 73348) where
incremental reflow doesn't work right in an |overflow: auto| <div>.

    
  
Blocks: 88690

    
  
Target Milestone: mozilla0.9.5 → mozilla0.9.6

    
    

    
  
Kin's reduced test case from bugscape bug 10622 which is the same as this bug

http://bugscape.netscape.com/showattachment.cgi?attach_id=3539

    
    

    
  
adding cc: list from bugscape bug 10622 <-- will close this one out.

This bug 91423 is the same bug.

    
    

    
  
I don't think this is a intl issue. Removing intl keyword. Nominating for the
next release.
Keywords: intl → nsbeta1

    
  
Whiteboard: after this is fixed verify URL works in bugscape 10622

    
  
Target Milestone: mozilla0.9.6 → mozilla0.9.7

    
    

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

    
    

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

    
    

    
  
As far as using MSN search as an example, does it still manifest the bug? It
works fine for me. 

    
    

    
  
They've changed the msn homepage quite a bit since this bug was filed. You can 
still see the bug with the testcases included above.

    
    

    
  
Dupe bug 104010 also has a good testcase there.


    
    

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

    
    

    
  
Using Win2K with Gecko/20011019 

On Priceline you can't rent a car, as the field doesn't let you enter 2 digits
to name your price. 

To replicate: enter 2 digits, backspace, type again etc.  
Another interesting thing is that if you paste in a session generated in IE, for
example, the 2 digits appear but then if you delete them and retype only 1 digit
may appear etc. 

It would be great to hear whether anyone thinks that is related to this bug or
if it looks like an evangelism issue. 

    
    

    
  
The priceline example is due to this bug, the only difference is that 
priceline specified a MAXLENGTH=2 on the textfield, which means you can't enter 
a 3rd character to make the 2nd one visible.

    
    

    
  
removing myself from the cc list.

    
    

    
  
Marked topembed. (Priceline is a top 200 site; in general it's not possible to
control when sites will code in a way that makes this bug appear.)
Keywords: topembed

    
    

    
  
Resummarizing, at susiew's request, to reflect what this bug is currently about.

Old Summary:

  "Typing in search field of msn homepage causes reflow and typing problem"

New Summary:

  "Every other typed char fails to render in a textfield wrapped in an inline 
style"
Summary: Typing in search field of msn homepage causes reflow and typing problem → Every other typed char fails to render in a textfield wrapped in an inline style

    
  
Component: Internationalization → Layout

    
    

    
  
evaughan's re-write of box-to-block adapter may do some good here. I'll apply
his patch and see if it makes a difference.
Depends on: 110328

    
    

    
  
Nope, evaughan's patch for bug 110328 (re-write of box-to-block adaptor) doesn't
fix the problem.

    
    

    
  
Maybe I'm restating the obvious here, but in doing development on our internal
website we've encountered this bug any time an INPUT item is embedded in a table
with specified WIDTH (either at the table or the cell level), and where the
INPUT was wrapped by _any_ formatting tag AND where text is printed prior to the
INPUT causing it to drop to the next line. If the close formatting tag is moved
to before the INPUT, the text displays properly. (i.e. 

<table width=100><tr><td><b>Hi there<input></b></td></tr></table> is buggy, while

<table width=100><tr><td><b>Hi there</b><input></td></tr></table> is fine.

    
    

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

    
  
Target Milestone: mozilla0.9.7 → mozilla0.9.8

    
    

    
  


  
This patch is a work in progress, but it illustrates the approach I'd like to
take to fixing the bug. To reiterate on my commments above, the problem here is
that an incremental reflow isn't reaching its target because the path to the
target contains continuation frames. These continuation frames are destroyed
during the unconstrained reflow that occurs to compute the max-width for the
table.

The fix I propose is to perform surgery on the reflow command, replacing the
continuation frames with their primaries, before doing the unconstrained
reflow. Note that we'll only do this for the inline frames along the path,
stopping when we encounter the first block frame.

This appears to fix the test case.

If this approach is okay, I'm going to modify the patch to expose the reflow
command's path directly to consumers: I think it would be best to keep this
path-fixup logic localized to the block frame as opposed to implementing it as
a magic method on the reflow command.

    
  
Keywords: patch

    
    

    
  


  
Here's how I'd like to fix it. The above patch won't apply unless you've
applied attachment 61632 [details] [diff] [review] the patch from bug 115113.

    
  
Depends on: 115113

    
    

    
  


  
Okay, screw the fancy C++: I was running into problems with |void
*&operator[]|. This just exposes the reflow path as a member on
nsHTMLReflowCommand.

        Attachment #61591 -
        Attachment is obsolete: true

        Attachment #61635 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 62085 [details] [diff] [review]
feh. let's keep it simple.

>+      // If this incremental reflow is targeted at a continuing frame,
>+      // we've got to retarget it to the primary frame. The
>+      // unconstrained reflow will destroy all of the continuations.
>+      if (aState.mNextRCFrame) {

It might be nice to stick an 
NS_ASSERTION(aState.GetFlag(BRS_ISINLINEINCRREFLOW),
	     "inline incremental reflow flag should be set")
right here, at least for documentation (since I think we
might want to consider replacing this optimization with
something that's faster and simpler, as I mentioned in email
recently).  (That should be true, right?)

r=dbaron

        Attachment #62085 -
        Flags: review+

    
    

    
  
+      // XXXwaterson if oldUnconstrainedWidth was set, why do we need
+      // to do the second reflow, below?

This might be related to bug 100103. I hit the assertion in bug 100103 all the
time and it is rather annoying.

    
    

    
  
For the record, dbaron was referring to a private email where he writes:

 * BRS_ISINLINEINCRREFLOW is too complex in that it exercises the system
   in ways that it wasn't, and probably doesn't need to be, designed
   for.  A frame state bit NS_FRAME_IS_REFLOW_ROOT would be simpler (we
   could remove *all* the BRS_ISINLINEINCRREFLOW code, which has a bunch
   of weird bugs that waterson has been working on), and faster.  The
   new frame state bit would indicate that the children of the frame
   have no effect on what is outside the frame and the frame can be
   called as the top-level recursive Reflow call when an incremental
   reflow of its contents is needed.

I absolutely think that this would be an elegant way to solve this problem. So
yeah, I'll add the assertion so we remember to undo this stuff.

    
    

    
  
Comment on attachment 62085 [details] [diff] [review]
feh. let's keep it simple.

sr=attinasi

        Attachment #62085 -
        Flags: superreview+

    
    

    
  
Fix checked in on the trunk.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch against 0.9.4 branch (obsolete)
        — 
          — Splinter Review
      

    


  
Here's a patch that's been back-ported to the 0.9.4 branch. It works, but the
BRS_ISINLINEINCRREFLOW assertion is tripping violently. I'll investigate a bit.

    
  
Keywords: edt0.9.4

    
  
Blocks: 116230

    
    

    
  
Chris -- Could you elaborate your nomination for 094 (What is driving this?).


    
    

    
  
I nominated this because it was a topembed; however, some related regressions
seem to be popping up (e.g., bug 116230), so I'll reneg on my nomination, if
that's okay.

    
    

    
  
Marking this fixed to get off EDT radar. However, it definitely caused bug
116230. I'll describe why in that bug.

    
    

    
  
Really marking fixed.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED

    
    

    
  
My perspective on why 094: Because this bug first was discovered on MSN then
Priceline as well as other sites, including one internal to AOL, it seems hard
to control where it might next appear...and when it does, it's nasty. From an
evangelism perspective, getting sites to code workarounds in a timely way (if at
all) will generally not be feasible. 

    
    

    
  
I still can reproduce the problem on the attached test case on 12-27-22-0.9.4ec
/ WinXP, but I don't it on recently trunk build.
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=48896

Re-open it for now.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

    
    

    
  
I don't think we're shooting to get this on the 0.9.4 branch anymore given that
the fix isn't all it was cracked up to be. Clearing `edt-0.9.4' keyword: is that
sufficient to make sure this won't be re-opened again?
Status: REOPENED → RESOLVED
Closed: 23 years ago → 23 years ago
Keywords: edt0.9.4
Resolution: --- → FIXED

    
    

    
  
Then I'll mark it as verified fixed - no reproduce on recently trunk build. 
Status: RESOLVED → VERIFIED

    
    

    
  
For the record, I found an instance of this bug on Microsoft.com. (Type in the 
search field/backspace/type more.)

http://office.microsoft.com/assistance/2000/Out2ksecFAQ.aspx





    
    

    
  
Just discovered this bug freezes/hangs Gecko/20011128, when you hit the
Backspace after typing in the field then try to continue typing where you left off.

You have to shut down the browser and restart. No TalkBack.

    
    

    
  


  
This patch incorporates the fix from bug 116230, and is back-ported to the
mozilla-0.9.4 branch.

        Attachment #62378 -
        Attachment is obsolete: true

    
    

    
  
dbaron, rbs, attinasi: could you re-review the latest patch, attachment 63992 [details] [diff] [review]?
It's a back-port of this fix, plus the fix in bug 116230, to the mozilla-0.9.4
branch. It's become a show-stopper for the mozilla-0.9.4 branch (for those who
can, see Bugscape bug 11618).

    
    

    
  
Comment on attachment 63992 [details] [diff] [review]
updated patch vs. 0.9.4 branch; incorporates fix for 116230

r=rbs

    
    

    
  
Comment on attachment 63992 [details] [diff] [review]
updated patch vs. 0.9.4 branch; incorporates fix for 116230

r=rbs

        Attachment #63992 -
        Flags: review+

    
    

    
  
Comment on attachment 63992 [details] [diff] [review]
updated patch vs. 0.9.4 branch; incorporates fix for 116230

sr=attinasi

        Attachment #63992 -
        Flags: superreview+

    
    

    
  
added edt0.9.4 keyword


Keywords: edt0.9.4

    
    

    
  
Fix checked in on mozilla-0.9.4 branch, a=chofmann.

    
  
Keywords: edt0.9.4 → edt0.9.4+, 
          
            fixed0.9.4

    
    

    
  
Is the fix on the trunk or in 0.9.8?
Bug 124639 is filed now and looks like a duplicate.

    
    

    
  
Removing dependency on unresolved bug 73348 since this verified bug obviously
doesn't actually depend on it.
No longer depends on: 73348
Depends on: 73348

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: