Closed Bug 106489 Opened 23 years ago Closed 23 years ago

[PATCH]Text entered in box does not appear

Categories

(Core :: Layout, defect, P3)

defect

Tracking

()

VERIFIED FIXED
mozilla1.0

People

(Reporter: throop, Assigned: kinmoz)

References

()

Details

(Keywords: testcase, Whiteboard: EDITORBASE+ [adt2])

Attachments

(5 files, 1 obsolete file)

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.5) Gecko/20011012
BuildID:    2001101202

Text entered in the 'Search' box on this website does not appear on the screen.
 It is actually entered, as can be seen by hitting the 'Go' button.  Works fine
on NS 4.75.

Reproducible: Always
Steps to Reproduce:
1.Go to http://www.alibris.com
2.Select the 'Search' box, and enter a search term
3.Hit 'Go'

Actual Results:  No text appears on the screen when typed.  However, the search
term entered does appear on the screen on the search results page after hitting
'Go'.

Expected Results:  The text should be displayed on the screen when entered.
doing the reduced test case now
Status: UNCONFIRMED → NEW
Ever confirmed: true
this is one weird problem. I have the reduced test case. If anything is removed 
from the test case, then it works. The leading table and trailing table must be 
there. I don't get it!
Attached file reduced test case
This sounds like a duplicate... I know I've seen it before (even the testcase 
looks very familiar)...

Kin: any ideas?

I don't think this is linux-specific
Rod, could you take a look? 
Assignee: attinasi → rods
Target Milestone: --- → mozilla0.9.9
very strange, since it isn't a sizing issue over to kin
Assignee: rods → kin
Moving this in to 0.9.7. A preliminary look at this in the debugger shows that
the floating table's TableOuterFrame origin isn't being set properly. That is,
it's y coordinate is zero during the incremental reflow when it should be
non-zero, so the TextWidget's view is not being positioned properly.

Still looking into where the problem is.
Status: NEW → ASSIGNED
OS: Linux → All
Priority: -- → P3
Hardware: PC → All
Whiteboard: EDITORBASE
Target Milestone: mozilla0.9.9 → mozilla0.9.7
*** Bug 105687 has been marked as a duplicate of this bug. ***
--> 0.9.8
Target Milestone: mozilla0.9.7 → mozilla0.9.8
confirming not platform specific. even though bug is already set to all, tested
using mac os x build 2001121805
Target Milestone: mozilla0.9.8 → mozilla0.9.9
plussing
Whiteboard: EDITORBASE → EDITORBASE+
Keywords: nsbeta1+
 Adding "testcase" keyword.
Keywords: testcase
--> mozilla1.0
Target Milestone: mozilla0.9.9 → mozilla1.0
Blocks: 108120
This is a log of an IRC discusssion kin and I had about this problem, and could
be useful for trying to remember what the problem was here.
Attached patch Patch Rev 1 (obsolete) — Splinter Review
Ok, so here's my first stab at fixing this particular problem.

The patch does the following:

  1. Moves GetRealFrame() and MoveChildrenTo() to a point in
     the file that is before anything that uses them *and* after
     the class nsFrameConstructorState is defined.

  2. Adds a method called AdjustOutOfFlowFrameParentPtrs()
     which is a method that recurses down through the frame
     hierarchy looking for placeholders within inline nodes.

  3. I modified MoveChildrenTo() to take an optional aState
     parameter, so that when it is present, would trigger
     a call to AdjustOutOfFlowFrameParentPtrs().

  4. Modified all calls to MoveChildrenTo() in the file
     to pass in a null, except when calling from ConstructInline().

To sum up the IRC discussion attatched to this bug, I found that
while loading the document, depending on the number of notifications
and when they happen the frame tree under an inline that contains
block elements is built differently, which I filed as bug 133989.

This meant that the path built by an nsHTMLReflowCommand could at
times be missing a parent to the target frame, which during reflow
could result in an IncrementalReflow being mapped to some other
type of reflow like a ResizeReflow which would somehow get ignored
because some of the frames didn't think there was any work that
needed to be done.

In any case this explains why I would see the problem with the
testcases *sometimes* and not other times.

This patch tries to correct or at least make the parent hierarchy
consistent, with the parent hierarchy that exists when things
do actually work.

Comments, reviews, and alternate approaches welcome.

dbaron - you'll probably notice that I simply reparent the out-of-flow
frame, and don't bother to move the out-of-flow that is in the aState
used in ConstructInline(). I'm a little fuzzy on what frame should
actually have the out-of-flow frame in their floater list in this
particular case.
Summary: Text entered in box does not appear → Text entered in box does not appear [adt2]
adt2
Summary: Text entered in box does not appear [adt2] → Text entered in box does not appear
Whiteboard: EDITORBASE+ → EDITORBASE+ [adt2]
Whiteboard: EDITORBASE+ [adt2] → EDITORBASE+ [adt2] FIX IN HAND, need r=, sr=, and a=
Comment on attachment 76607 [details] [diff] [review]
Patch Rev 1

A comment about what this function does would be nice. :-)

>+static void
>+AdjustOutOfFlowFrameParentPtrs(nsIPresContext*          aPresContext,
>+                               nsIFrame*                aFrame,
>+                               nsFrameConstructorState* aState)
>+{
>+  nsIFrame *outOfFlowFrame = GetRealFrame(aFrame);
>+
>+  if (outOfFlowFrame && outOfFlowFrame != aFrame) {
>+
>+    // Get the display data for the outOfFlowFrame so we can
>+    // figure out if it is a floater or absolutely positioned element.
>+
>+    const nsStyleDisplay* display = nsnull;
>+    outOfFlowFrame->GetStyleData(eStyleStruct_Display,
>+                                (const nsStyleStruct*&)display);
>+ 
>+    if (!display) {
>+      NS_WARNING("outOfFlowFrame has no display data!");
>+      return;
>+    }
>+
>+    // Update the parent pointer for outOfFlowFrame if it's
>+    // containing block has changed as the result of reparenting,
>+    //
>+    // XXX_kin: I don't think we have to worry about
>+    // XXX_kin: NS_STYLE_POSITION_FIXED or NS_STYLE_POSITION_RELATIVE.
>+
>+    if (NS_STYLE_POSITION_ABSOLUTE == display->mPosition) {
>+      // XXX_kin: I think we'll need to add code here to handle the
>+      // XXX_kin: reparenting that can happen in ConstructInline()?
>+      // XXX_kin:
>+      // XXX_kin: The case I'm thinking about here is when the inline being
>+      // XXX_kin: constructed has a style="position: relative;" property
>+      // XXX_kin: on it, and ConstructInline() moves/reparents all child block
>+      // XXX_kin: frames and any inlines (including placeholders) that happen
>+      // XXX_kin: to be between these blocks, under the new inline-block it created.
>+      // XXX_kin: I think right now this case generates an assertion during
>+      // XXX_kin: reflow, and as a result things fail to render since I believe
>+      // XXX_kin: the placeholder is parented to the inline-block, and the
>+      // XXX_kin: outOfFlowFrame is in the original inline frame's absolute list,
>+      // XXX_kin: and is also parented to it.
>+    }
>+    else if (NS_STYLE_FLOAT_NONE != display->mFloats) {
>+      outOfFlowFrame->SetParent(aState->mFloatedItems.containingBlock);
>+    }
>+
>+    // XXX_kin: Hmmmm, do we have to worry about placeholders within
>+    // XXX_kin: out-of-flow frames? If so, we'll probably want to continue
>+    // XXX_kin: here instead of just returning.
>+

I think it might be more complicated, even. :-/

Specifically, we'd want to continue to descend into placeholders until we found
an
out-of-flow frame that established a new containing block for absolutely
positioned
elements. At that point, we could terminate the descent, because any absolutely
positioned frames below that frame would be properly parented.

But we can fix that another day.


>@@ -14198,7 +14290,7 @@
>                       nsnull, blockSC, nsnull, blockFrame);
> 
>   blockFrame->SetInitialChildList(aPresContext, nsnull, aBlockChildFrame);
>-  MoveChildrenTo(aPresContext, blockSC, blockFrame, aBlockChildFrame);
>+  MoveChildrenTo(aPresContext, blockSC, blockFrame, aBlockChildFrame, nsnull);
> 
>   // Create an anonymous inline frame that will parent
>   // aRightInlineChildFrame. The new frame won't have a parent yet:

Does this case potentially need to re-parent out-of-flow frames, too? (I.e.,
should
we be passing in a state instead of nsnull?)

r=/sr=waterson.
Attachment #76607 - Flags: superreview+
Attached patch Patch Rev 1.1Splinter Review
This patch adds the comment requested by waterson, as well as incorporates his
comments about future work to support recursive descent into placeholders.

To answer waterson's question:

> Does this case potentially need to re-parent out-of-flow frames, too? (I.e.,
> should we be passing in a state instead of nsnull?)

At the moment my patch only deals with floaters. Since we are reparenting the
items in list3 to another inline frame, that doesn't result in any containment
block change for floaters in the list, so passing in a state would still do
nothing with the current implementation. I probably will have to pass a state
once AdjustOutOfFlowFrameParentPtrs() supports absolute positioned frames.
Attachment #76607 - Attachment is obsolete: true
Attachment #77936 - Flags: superreview+
Summary: Text entered in box does not appear → [PATCH]Text entered in box does not appear
Whiteboard: EDITORBASE+ [adt2] FIX IN HAND, need r=, sr=, and a= → EDITORBASE+ [adt2] FIX IN HAND, need r=, and a=
Comment on attachment 77936 [details] [diff] [review]
Patch Rev 1.1

r=attinasi
Attachment #77936 - Flags: review+
Whiteboard: EDITORBASE+ [adt2] FIX IN HAND, need r=, and a= → EDITORBASE+ [adt2] FIX IN HAND, need a=
Answering waterson's question again, this time paying attention to the last arg 
in his example. ;-)

> Does this case potentially need to re-parent out-of-flow frames, too? (I.e.,
> should we be passing in a state instead of nsnull?)

I imagine it would if we are reparenting to a block frame, though I'm not 
exactly sure how to generate a test case that would trigger this particular 
piece of code. I thought it would be more prudent at this time to just leave it 
alone to avoid any potential regressions and perhaps revisit it when bug 133989 
is addressed.
Whiteboard: EDITORBASE+ [adt2] FIX IN HAND, need a= → EDITORBASE+ [adt2] FIX IN HAND, need a= (waiting for driver + adt approval)
Keywords: adt1.0.0
Removing adt1.0.0. Pls land this on the trunk, and let it bake for a couple of
days. If there are no regressions, and QA approves, pls renominate.

Does this this bug manifest itself only on this site, or does it appear on any
top 100 sites?
Keywords: adt1.0.0
adt: Although we have not identified other specific sites, the condition under
which text fields will fail is not uncommon. The text field just needs to be
inside a floater (align=) container.  Floaters are very common, so we would
expect to see a fairly large number sites that exhibit this flaw.
Fix checked in on TRUNK:

    mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp  revision 1.729

We'll let this bake a couple days on the TRUNK before I re-request adt and 
driver approval for checkin on the MOZILLA_1_0_0_BRANCH.
Whiteboard: EDITORBASE+ [adt2] FIX IN HAND, need a= (waiting for driver + adt approval) → EDITORBASE+ [adt2][FIXED_ON_TRUNK] FIX IN HAND, need a= (waiting for driver + adt approval)
petersen: Could you verify the fix on the trunk? Thanks!
Verified on OS X trunk (2002-04-12-08) and Windows ME Trunk (2002-04-12-10)
Keywords: adt1.0.0
Resolving as FIXED. Please add fixed1.0.0 keyword after the fix has been checked
into the 1.0.0 branch.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Based on my last comment, marking verified.
Status: RESOLVED → VERIFIED
adt1.0.0+ (on ADT's behalf) for checkin into the 1.0 branch. Pls check this in
to the branch today. After it is checked in, pls add fixed1.0.0. Once QA has
verified it on the branch, then add verified1.0.0.
Keywords: adt1.0.0adt1.0.0+
Whiteboard: EDITORBASE+ [adt2][FIXED_ON_TRUNK] FIX IN HAND, need a= (waiting for driver + adt approval) → EDITORBASE+ [adt2][FIXED_ON_TRUNK] FIX IN HAND, need a= (waiting for driver)
Keywords: approval
Comment on attachment 77936 [details] [diff] [review]
Patch Rev 1.1

a=rjesup@wgate.com for post-RC1 checkin
Attachment #77936 - Flags: approval+
Fixed on the MOZILLA_1_0_0_BRANCH:

    mozilla/layout/html/style/src/nsCSSFrameConstructor.cpp  revision 1.727.2.2
Keywords: fixed1.0.0
Whiteboard: EDITORBASE+ [adt2][FIXED_ON_TRUNK] FIX IN HAND, need a= (waiting for driver) → EDITORBASE+ [adt2]
Verified in branch (0SX 2002-04-26-05) and (WinME 2002-04-25-06).
Keywords: verified1.0.0
I have experienced not being able to enter text into text fields with several
Web sites in 0.99. However, the problem seemed to have been resolved with 1.0
RC1.  Now, I am experiencing this problem again with 1.0 RC2.

Steps to reproduce this bug:
1.  Go to http://www.leedungarees.com/ .  You will need Flash to access this site.
2.  Click on the "Register to Win Jeans!" Flash link.
3.  A pop-up window will appear.  Try to enter text in the "Email" text field.


*** Bug 116857 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: