50758 - INPUT TYPE="text" STYLE="text-align: right;" first char typed are not displayed

Status: VERIFIED FIXED

(Core :: Layout: Form Controls, defect, P3)

Description

[Vicente Salvador]

<HTML>
<BODY>
<FORM NAME="aForm">
<INPUT TYPE="text" NAME="bField" STYLE="text-align:right;">
</FORM>
</BODY>
</HTML>

THIS CODE ALIGN TEXT CORRECTLY but first char typed are showed in the form as a 
blank space.

Let's see a sample...

If you type string: 12345 then you see only 2345 in the field. If you reload the 
page then correct string: 12345 can be seen in the input. Also javascript value
is ok, so it seems that it's only a visualization issue.

If you press backspace many times, you can see that cursor goes at the left of
the input field and desapears showing on the top-left (outside the field).

Then, press a key (the number 1) and see that this number doesn't appears in
the input but the cursor moves inside the field and appears on the right hand.
Then press the number 2 and see that this number appears because the cursor
is alreadly in the correct possition.

Press the left cursor twice and see that the posicion when number 1 should 
appear has an space (?)

This test hass been performed on Windows 2000 Mozilla build: 2000082313

Comment 1

[Vicente Salvador]

Attachment: Create an input field with right alignement

Comment 2

[rods (gone)]

I don't know if this is an editor issue or a layout issue.

Comment 3

[rubydoo123]

talked with Kin, seems this one is a layout issue, handing over to buster

Comment 4

[ckritzer (gone)]

PDT: Nominating nsbet3+
Reason: Perceived loss of data; high visibility in forms

Comment 5

[ckritzer (gone)]

Attachment: attachment from bug 50758

Comment 6

[ckritzer (gone)]

oops...that last attachment should read:

"attachment from bug#51700"

Comment 7

[ckritzer (gone)]

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

Comment 8

[buster]

Looking at this now.

Comment 9

[buster]

I'm testing a workaround for this now.  Basically, I detect the case of 
right-aligned inline elements, and invalidate the line.  Typing into 
a right-aligned text control makes the text frame grow RTL.  Incremental 
invalidation works fine for LTR, but today it's weak for RTL. This fix gets us 
over that hump.  

Editor dudes:  even with my fix, the first character typed in an empty 
right-aligned text control doesn't show up.  But all subsequent typing repaints 
the entire string properly with my fix.  If I use viewer and dump frames after 
the first character is inserted, I don't see the character in the frame model at 
all.  Is this a known issue?  Should I submit a separate bug?

Comment 10

[buster]

I have a fix-in-hand.  Low risk.  Should be nsbeta3+

Comment 11

[buster]

I need a priority assessment of this bug from PDT.  
Behavior without my fix:  typing in text fields that have text-align: right does 
not show up.  The content is there, but the invalidate calls are wrong, so 
obscuring the window and forcing a repaint typically displays the typed text.
With my fix, this displays correctly in all cases except what I asked the editor 
folks to look into in my previous comment.
Very low risk, just extra invalidate calls that are constrained to the content 
of the text control.
Moderate value:  enables the use of right-aligned text in a text control (note, 
that's the text inside the control that is right aligned, not the control 
itself.)  Nav4 didn't support this, so there's no legacy issue with our own 
product, but IE 5.5 for Windows does support this properly.

If this fix isn't accepted, we should disable right-aligning of text inside edit 
controls.

Comment 12

[karnaze (gone)]

Marking nsbeta3+ since the risk is low and the alternative of disabling right 
aligned of text may have risk.

Comment 13

[buster]

Attachment: proposed fix: extra invalidation for right-aligned text

Comment 14

[buster]

Need a review.  chris k. or chris w? 
I also need input from editor team about why the first character doesn't show 
up. To see what I mean, open first test case in viewer, type a single char, and 
dump frames.  There is no character in the frame dump.  Type a second character, 
they both display, and if you dump frames they're both there in the frame tree.  
Is this the same problem don cone ran into with printing of text control 
content?  In any event, I think that's a separate problem from the one I fixed.

Comment 15

[Simon Fraser [no longer active]]

The diff looks ok. Maybe Joe knows about the 'missing first char' problem?

Comment 16

[buster]

fix checked in

Comment 17

[bsharma]

Updating QA contact.

Comment 18

[bsharma]

Build: 2000092711 MN6
Platform: WinNT

Verified with the attached 1st testcase.

Comment 19

[Adam Kennedy]

Attachment: Additional Test Case with Javascript

Comment 20

[Adam Kennedy]

This isn't fixed...

An additional note, if you set the value from Javascript, it isn't just the
first character. (test case attached)

Comment 21

[Joe Francis]

i'll try to take a look at the first character issue...

Comment 22

[James Lariviere]

It has been reported today (bug 60448) that this bug has regressed. :-(

Comment 23

[James Lariviere]

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

Comment 24

[Fabian Guisset]

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

Comment 25

[Jesse Ruderman]

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

Comment 26

[Jesse Ruderman]

Several dups and also several possible dups (bug 51703 and bug 64414), so 
nominating for nsbeta1.

Comment 27

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

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

Comment 28

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

Could this be related to bug 45562?

Comment 29

[rbs]

After checking, I haven't seen a connection between this bug and bug 45562. What 
I instead saw regarding this bug is that when characters other than the first 
character are typed in, the GfxText frame is reflowed with reason=incremental,
that's alright; In contrast however, when the 1st character is typed in, the
GfxText frame is reflowed with a reason=resize --and this is having some
undesirable side-effects. Indeed, due to some optimized reflow paths somewhere
in the box system, the very first initial reflow *never* reaches the newly 
created inner nsTextFrame (and this isn't invalidated and painted). I saw this
because I put some print statements in nsTexFrame::Reflow() and they didn't
print when intended. And to confirm this, I tried to forcibly set
reason=incremental in GfxText::Reflow(). With this I could immediately see the
correct display of the first character (as well as of the JS example).

In reality, when the first character is typed in, a reflow is dispatched
with reason=incremental and with target on the surrounding <p>, but this reason
gets changed later in nsLineLayout::ReflowFrame(). For other characters, the
target is the inner nsTextFrame and it now doesn't miss its reflow. The excerpt
below starts with line 797 in which reason=resize is taken as the default. Then
none of the if clauses apply for the situation of a GfxText with a newly created
nsTextFrame for the first character. Thus the reflow for this is propagated on
line 919 with reason=resize. (NB: When characters are deleted, the corresponding
nsTextFrame and content node are deleted, hence when a new first character is
again typed in, the same process I descrived above repeats itself.)

745 nsresult
746 nsLineLayout::ReflowFrame(nsIFrame* aFrame,
747                           nsIFrame** aNextRCFrame,
748                           nsReflowStatus& aReflowStatus,
749                           nsHTMLReflowMetrics* aMetrics,
750                           PRBool& aPushedFrame)
751 {
[ ... ]
792   // Get reflow reason set correctly. It's possible that a child was
793   // created and then it was decided that it could not be reflowed
794   // (for example, a block frame that isn't at the start of a
795   // line). In this case the reason will be wrong so we need to check
796   // the frame state.
797   nsReflowReason reason = eReflowReason_Resize;
798   nsFrameState state;
799   aFrame->GetFrameState(&state);
800   if (NS_FRAME_FIRST_REFLOW & state) {
801     reason = eReflowReason_Initial;
802   }  
803   else if (*aNextRCFrame == aFrame) {
804     reason = eReflowReason_Incremental;
805     // Make sure we only incrementally reflow once
806     *aNextRCFrame = nsnull;
807   }
808   else if (psd->mReflowState->reason == eReflowReason_StyleChange) {
809     reason = eReflowReason_StyleChange;
810   }
811   else if (psd->mReflowState->reason == eReflowReason_Dirty) {
812     if (state & NS_FRAME_IS_DIRTY)
813       reason = eReflowReason_Dirty;
814   }
815   else {
816     const nsHTMLReflowState* rs = psd->mReflowState;
817     if (rs->reason == eReflowReason_Incremental) {
818       // If the incremental reflow command is a StyleChanged reflow and
819       // it's target is the current span, then make sure we send
820       // StyleChange reflow reasons down to the children so that they
821       // don't over-optimize their reflow.
822       nsIReflowCommand* rc = rs->reflowCommand;
823       if (rc) {
824         nsIReflowCommand::ReflowType type;
825         rc->GetType(type);
826         if (type == nsIReflowCommand::StyleChanged) {
827           nsIFrame* parentFrame = psd->mFrame
828             ? psd->mFrame->mFrame
829             : mBlockReflowState->frame;
830           nsIFrame* target;
831           rc->GetTarget(target);
832           if (target == parentFrame) {
833             reason = eReflowReason_StyleChange;
834           }
835         }
836         else if (type == nsIReflowCommand::ReflowDirty &&
837                  (state & NS_FRAME_IS_DIRTY)) {          
838           reason = eReflowReason_Dirty;
839         }
840       }
841     }
842   }
843 
844   // Setup reflow state for reflowing the frame
845   nsHTMLReflowState reflowState(mPresContext, *psd->mReflowState,
846                                 aFrame, availSize,
847                                 (psd->mRightEdge - psd->mLeftEdge),
848                                 psd->mReflowState->availableHeight);
849   reflowState.reason = reason;
850   reflowState.mLineLayout = this;
[ ... ]
919   aFrame->Reflow(mPresContext, metrics, reflowState, aReflowStatus);
[ ... ]
1171   return rv;
1172 }

Comment 30

[rbs]

While the optimizations go well in LTR mode, something puzzling is happening in 
RTL mode.

Comment 31

[rbs]

Got some further insights into what is happening. The reason why the initial
reflow doesn't reach the nsTextFrame is due to the optimization on 
line 772 of nsBlockToBox::Reflow code as indicated below.

The code uses a 'needsReflow' flag to decide whether or not to propagate the 
reflow. In normal RTL (style="text-align:left"), the 'if' clause fails, the flag
is set and the reflow is propagated. But in LTR (style="text-align:right"), the
'if' clause succeeds, and the flag isn't set and the reflow isn't propagated.
(Note: we get to that point if reason=resize, so this doesn't capture other
characters for which the reflow is dispatched with reason=incremental and the 
target nsTextFrame as I indicated earlier).

The mLastSize under consideration is set from the aDesiredSize from the previous
reflow, and thus it is the current size of the nsBox frame which is interacting
here.

In LTR mode, the nsBox frame has a size as wide as the input field (due to style
constraints in its child frame), and thus it is *constant*.

In RTL mode, the size grows as more characters are typed in.

That's why they fall under different code paths at that 'if' juncture, causing
the 1st character in LTR not to be reflowed and displayed. I think this a fish
for evaughan to fry...

578 nsresult
579 nsBoxToBlockAdaptor::Reflow(nsBoxLayoutState& aState,
580                      nsIPresContext*   aPresContext,
581                      nsHTMLReflowMetrics&     aDesiredSize,
582                      const nsHTMLReflowState& aReflowState,
583                      nsReflowStatus&          aStatus,
584                      nscoord aX,
585                      nscoord aY,
586                      nscoord aWidth,
587                      nscoord aHeight,
588                      PRBool aMoveFrame)
589 {
[ ... ]
625   PRBool needsReflow = PR_FALSE;
626 
627   // get our frame its size and the reflow state
628   nsReflowReason reason = aReflowState.reason;
[ ... ]
752   // if we don't need a reflow then 
753   // lets see if we are already that size. Yes? then don't even reflow. We 
are done.
754   if (!needsReflow) {
755       
756       if (aWidth != NS_INTRINSICSIZE && aHeight != NS_INTRINSICSIZE) {
757       
758           // if the new calculated size has a 0 width or a 0 height
759           if ((mLastSize.width == 0 || mLastSize.height == 0) && (aWidth == 
0 || aHeight == 0)) {
760                needsReflow = PR_FALSE;
761                aDesiredSize.width = aWidth; 
762                aDesiredSize.height = aHeight; 
763                mFrame->SizeTo(aPresContext, aDesiredSize.width, 
aDesiredSize.height);
764           } else {
765             aDesiredSize.width = mLastSize.width;
766             aDesiredSize.height = mLastSize.height;
767 
768             // remove the margin. The rect of our child does not include it 
but our calculated size does.
769             nscoord calcWidth = aWidth; 
770             nscoord calcHeight = aHeight; 
771             // don't reflow if we are already the right size
772             if (mLastSize.width == calcWidth && mLastSize.height == 
calcHeight)
773 >>> LTR >>>       needsReflow = PR_FALSE;
774             else
775 >>> RTL >>>       needsReflow = PR_TRUE;
776    
777           }
778       } else {
779           // if the width or height are intrinsic alway reflow because
780           // we don't know what it should be.
781          needsReflow = PR_TRUE;
782       }
783   }
784                              
785   // ok now reflow the child into the springs calculated space
786   if (needsReflow) {

[ ... ]
994   return NS_OK;
995 }

Comment 32

[rbs]

Re-assigning to evaughan per comments above.

Comment 33

[Peter Trudelle]

->moz1.1

Comment 34

[Fabian Guisset]

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

Comment 35

[Vladimir Ermakov]

QA Contact Update

Comment 36

[Boris Zbarsky [:bzbarsky]]

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

Comment 37

[rbs]

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

Comment 38

[Andreas M. "Clarence" Schneider]

This seems not to be limited to RTL mode. Note related bug 59909.

Comment 39

[Joe Kelsey]

This appears to be fixed in 0.9.1.

I just visited a problematic table (right-justified entry boxes) and found that
the problem has disappeared!

You may want to verify with others...

Comment 40

[rbs]

Yes, it seemed to be gone. The only remaining problem is that the caret doesn't
show up the first time.

evaughan, was the "needsReflow" optimization relaxed?

Comment 41

[Vicente Salvador]

Yes, it seemed to be resolved. The problem with the caret maybe can be
considered another bug

Comment 42

[Christopher Aillon (sabbatical, not receiving bugmail)]

I don't see the initial problem this bug was about.  However, I do see the
problem with the cursor not displaying.  I have filed bug 105397 regarding that
issue.

Since I don't see the problem anymore, and the last couple of comments also
noted they didn't either, I'm marking this fixed.  Please see bug 105397 for the
cursor issue.

Comment 43

[Vladimir Ermakov]

verifying fixed on windows 98 2002-01-07-03-trunk