If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

VERIFIED FIXED in mozilla0.9.8

Status

()

Core
Layout
P2
normal
VERIFIED FIXED
16 years ago
16 years ago

People

(Reporter: Yuying Long, Assigned: Chris Waterson)

Tracking

({testcase, topembed})

Trunk
mozilla0.9.8
testcase, topembed
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(6 attachments, 3 obsolete attachments)

(Reporter)

Description

16 years ago
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.
(Reporter)

Comment 1

16 years ago
Created attachment 42807 [details]
Screen shot of shifted search text field.
(Reporter)

Updated

16 years ago
Keywords: intl
QA Contact: andreasb → ylong

Comment 2

16 years ago
I saw the screen shot and tried it on my machine but I am not sure what is the
problem.
(Reporter)

Comment 3

16 years ago
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.

Comment 4

16 years ago
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

Comment 5

16 years ago
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
(Reporter)

Comment 6

16 years ago
Not reproduce on N4.7.

Comment 7

16 years ago
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

Comment 8

16 years ago
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

Comment 9

16 years ago
-->kin
Assignee: beppe → kin

Comment 10

16 years ago
Created attachment 47366 [details]
Minimal test case.

Comment 11

16 years ago
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
(Assignee)

Comment 12

16 years ago
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
(Assignee)

Comment 13

16 years ago
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
(Assignee)

Comment 14

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

Comment 15

16 years ago
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.
(Assignee)

Comment 16

16 years ago
Created attachment 48896 [details]
minimal test with constrained <td> width to force-wrap
(Assignee)

Updated

16 years ago
OS: Windows 2000 → All
Hardware: PC → All
(Assignee)

Updated

16 years ago
Depends on: 73348
(Assignee)

Comment 17

16 years ago
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>.
(Assignee)

Updated

16 years ago
Blocks: 88690
(Assignee)

Updated

16 years ago
Target Milestone: mozilla0.9.5 → mozilla0.9.6

Comment 18

16 years ago
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

Comment 19

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

This bug 91423 is the same bug.

Comment 20

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

Updated

16 years ago
Whiteboard: after this is fixed verify URL works in bugscape 10622
(Assignee)

Updated

16 years ago
Target Milestone: mozilla0.9.6 → mozilla0.9.7

Comment 21

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

Comment 22

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

Comment 23

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

Comment 24

16 years ago
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.

Comment 26

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

Comment 27

16 years ago
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. 

Comment 28

16 years ago
Created attachment 59160 [details]
Source code for Priceline car rental "name your price" page

Comment 29

16 years ago
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.

Comment 30

16 years ago
removing myself from the cc list.

Comment 31

16 years ago
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

Comment 32

16 years ago
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

Updated

16 years ago
Component: Internationalization → Layout

Comment 33

16 years ago
Changed URL from msn.com to
http://www.priceline.com/rentalcars/lang/en-us/itinerary.asp
(Assignee)

Comment 34

16 years ago
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

Comment 35

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

Comment 36

16 years ago
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.

Comment 37

16 years ago
*** Bug 112681 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

16 years ago
Target Milestone: mozilla0.9.7 → mozilla0.9.8
(Assignee)

Comment 38

16 years ago
Created attachment 61591 [details] [diff] [review]
fix-up to incremental reflow command before performing unconstrained reflow

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.
(Assignee)

Updated

16 years ago
Keywords: patch
(Assignee)

Comment 39

16 years ago
Created attachment 61635 [details] [diff] [review]
diff version tree patched with changes from bug 115113

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.
(Assignee)

Updated

16 years ago
Depends on: 115113
(Assignee)

Comment 40

16 years ago
Created attachment 62085 [details] [diff] [review]
feh. let's keep it simple.

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+

Comment 42

16 years ago
+      // 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.
(Assignee)

Comment 43

16 years ago
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 44

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

sr=attinasi
Attachment #62085 - Flags: superreview+
(Assignee)

Comment 45

16 years ago
Fix checked in on the trunk.
(Assignee)

Comment 46

16 years ago
Created attachment 62378 [details] [diff] [review]
patch against 0.9.4 branch

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.
(Assignee)

Updated

16 years ago
Keywords: edt0.9.4
(Assignee)

Updated

16 years ago
Blocks: 116230

Comment 47

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

Comment 48

16 years ago
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.
(Assignee)

Comment 49

16 years ago
Marking this fixed to get off EDT radar. However, it definitely caused bug
116230. I'll describe why in that bug.
(Assignee)

Comment 50

16 years ago
Really marking fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 51

16 years ago
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. 
(Reporter)

Comment 52

16 years ago
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 → ---
(Assignee)

Comment 53

16 years ago
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
Last Resolved: 16 years ago16 years ago
Keywords: edt0.9.4
Resolution: --- → FIXED
(Reporter)

Comment 54

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

Comment 55

16 years ago
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



Comment 56

16 years ago
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.
(Assignee)

Comment 57

16 years ago
Created attachment 63992 [details] [diff] [review]
updated patch vs. 0.9.4 branch; incorporates fix for 116230

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
(Assignee)

Comment 58

16 years ago
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 59

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

r=rbs

Comment 60

16 years ago
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 61

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

sr=attinasi
Attachment #63992 - Flags: superreview+

Comment 62

16 years ago
added edt0.9.4 keyword

Keywords: edt0.9.4
(Assignee)

Comment 63

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

Updated

16 years ago
Keywords: edt0.9.4 → edt0.9.4+, fixed0.9.4

Comment 64

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

Comment 65

16 years ago
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.