46396 - select after input confuses session history

Status: VERIFIED FIXED

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

Description

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

DESCRIPTION:  A select after an input  shows its initial value after submitting
a form with it changed and hitting back, and is changed to the changed value
after the form is reset.

STEPS TO REPRODUCE:
 1) load attached testcase
 2) Type text in the input
 3) Change the select to "Two"
 4) Submit the form
 5) Hit back
 6) Hit Reset on the form

ACTUAL RESULTS:
 after 5, the select shows One
 after 6, the select shows Two

EXPECTED RESULTS:
 after 5, the select shows Two
 after 6, the select shows One

DOES NOT WORK CORRECTLY ON:
 * Linux, mozilla, 2000-07-21-13-M17

Comment 1

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

Attachment: testcase

Comment 2

[Kevin McCluskey (gone)]

Confirmed that it does not work properly on WINNT.
Moving to M22.

Comment 3

[rods (gone)]

I need to research this somemore

Comment 4

[Kevin McCluskey (gone)]

Marking nsbeta3-

Comment 5

[Kevin McCluskey (gone)]

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

Comment 6

[Kevin McCluskey (gone)]

Marking nsbeta3+ based on real world example where this is causing problem see  
Bug 46695 

Comment 7

[Kevin McCluskey (gone)]

Setting priority to P1

Comment 8

[rods (gone)]

The problem is with the GfxText, in the RestoreState method it calls:

  nsresult res = SetProperty(aPresContext, nsHTMLAtoms::value, stateString);

Which ends up Flushing the Pending notifications, which means the 
ListControlFrame get it's initial reflow before it gets it's PresState value 
set.

I think the GfxText needs to cache the PresState until it gets it's initial 
reflow and then do the appropriate thing or make it so it doesn't Flush the 
pending notifications thus causing a reflow on combobox before it gets it's 
PresState.

One could argue that the combo/listbox should be able to deal with getting it's 
PresState set after the initial reflow, but I think that is bizarre and not the 
intent of how the PresState should work. Also, for me to fix it it I would then 
have to flush the pendng notifications. This means a lot of extra reflows from 
the top of the document down to the gfxText and select.

reassigning to editor

The following is a stack tracing show how the combobox gets reflowed from the 
GfxText RestoreState:


nsListControlFrame::Reset(nsIPresContext * 0x02a51dc0) line 1813
nsListControlFrame::Reflow(nsListControlFrame * const 0x0114538c, nsIPresContext 
* 0x02a51dc0, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, 
unsigned int & 0) line 486
nsContainerFrame::ReflowChild(nsIFrame * 0x0114538c, nsIPresContext * 
0x02a51dc0, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int 0, 
int 0, unsigned int 21, unsigned int & 0) line 693 + 31 bytes
nsComboboxControlFrame::ReflowComboChildFrame(nsIFrame * 0x0114538c, 
nsIPresContext * 0x02a51dc0, nsHTMLReflowMetrics & {...}, const 
nsHTMLReflowState & {...}, unsigned int & 0, int 1073741824, int 1073741824) 
line 631 + 44 bytes
nsComboboxControlFrame::Reflow(nsComboboxControlFrame * const 0x011452a0, 
nsIPresContext * 0x02a51dc0, nsHTMLReflowMetrics & {...}, const 
nsHTMLReflowState & {...}, unsigned int & 0) line 1389
nsLineLayout::ReflowFrame(nsIFrame * 0x011452a0, nsIFrame * * 0x0012d154, 
unsigned int & 0, nsHTMLReflowMetrics * 0x00000000, int & 0) line 937
nsBlockFrame::ReflowInlineFrame(nsBlockReflowState & {...}, nsLineLayout & 
{...}, nsLineBox * 0x0114ee88, nsIFrame * 0x011452a0, unsigned char * 
0x0012c6cc) line 4336 + 29 bytes
nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState & {...}, nsLineLayout & 
{...}, nsLineBox * 0x0114ee88, int * 0x0012cd48, unsigned char * 0x0012cb90, int 
0, int 0) line 4220 + 28 bytes
nsBlockFrame::DoReflowInlineFramesAuto(nsBlockReflowState & {...}, nsLineBox * 
0x0114ee88, int * 0x0012cd48, unsigned char * 0x0012cb90, int 0, int 0) line 
4156 + 42 bytes
nsBlockFrame::ReflowInlineFrames(nsBlockReflowState & {...}, nsLineBox * 
0x0114ee88, int * 0x0012cd48, int 0, int 0) line 4101 + 32 bytes
nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineBox * 0x0114ee88, int 
* 0x0012cd48, int 0) line 3236 + 29 bytes
nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}) line 2925 + 27 bytes
nsBlockFrame::Reflow(nsBlockFrame * const 0x011fe0dc, nsIPresContext * 
0x02a51dc0, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, 
unsigned int & 0) line 1729 + 15 bytes
nsBlockReflowContext::DoReflowBlock(nsHTMLReflowState & {...}, nsReflowReason 
eReflowReason_Initial, nsIFrame * 0x011fe0dc, const nsRect & {x=0 y=285 
width=8940 height=1073741824}, int 1, int 240, int 0, nsMargin & {top=0 right=0 
bottom=0 left=0}, unsigned int & 0) line 561 + 36 bytes
nsBlockReflowContext::ReflowBlock(nsIFrame * 0x011fe0dc, const nsRect & {x=0 
y=285 width=8940 height=1073741824}, int 1, int 240, int 0, nsMargin & {top=0 
right=0 bottom=0 left=0}, unsigned int & 0) line 331 + 50 bytes
nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & {...}, nsLineBox * 
0x0114ee60, int * 0x0012d880) line 3854 + 56 bytes
nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineBox * 0x0114ee60, int 
* 0x0012d880, int 1) line 3118 + 23 bytes
nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}) line 2925 + 27 bytes
nsBlockFrame::Reflow(nsBlockFrame * const 0x011fdfa4, nsIPresContext * 
0x02a51dc0, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, 
unsigned int & 0) line 1729 + 15 bytes
nsBlockReflowContext::DoReflowBlock(nsHTMLReflowState & {...}, nsReflowReason 
eReflowReason_Incremental, nsIFrame * 0x011fdfa4, const nsRect & {x=0 y=0 
width=9180 height=1073741824}, int 1, int 0, int 1, nsMargin & {top=0 right=0 
bottom=0 left=0}, unsigned int & 0) line 561 + 36 bytes
nsBlockReflowContext::ReflowBlock(nsIFrame * 0x011fdfa4, const nsRect & {x=0 y=0 
width=9180 height=1073741824}, int 1, int 0, int 1, nsMargin & {top=0 right=0 
bottom=0 left=0}, unsigned int & 0) line 331 + 50 bytes
nsBlockFrame::ReflowBlockFrame(nsBlockReflowState & {...}, nsLineBox * 
0x011fdff0, int * 0x0012e3b8) line 3854 + 56 bytes
nsBlockFrame::ReflowLine(nsBlockReflowState & {...}, nsLineBox * 0x011fdff0, int 
* 0x0012e3b8, int 1) line 3118 + 23 bytes
nsBlockFrame::ReflowDirtyLines(nsBlockReflowState & {...}) line 2925 + 27 bytes
nsBlockFrame::Reflow(nsBlockFrame * const 0x011fdf58, nsIPresContext * 
0x02a51dc0, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, 
unsigned int & 0) line 1729 + 15 bytes
nsContainerFrame::ReflowChild(nsIFrame * 0x011fdf58, nsIPresContext * 
0x02a51dc0, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int 0, 
int 0, unsigned int 0, unsigned int & 0) line 693 + 31 bytes
CanvasFrame::Reflow(CanvasFrame * const 0x011fd284, nsIPresContext * 0x02a51dc0, 
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) 
line 306
nsBoxToBlockAdaptor::Reflow(nsBoxLayoutState & {...}, nsIPresContext * 
0x02a51dc0, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, 
unsigned int & 0, int 0, int 0, int 9180, int 4470, int 1) line 813
nsBoxToBlockAdaptor::DoLayout(nsBoxToBlockAdaptor * const 0x011fdef0, 
nsBoxLayoutState & {...}) line 484 + 52 bytes
nsBox::Layout(nsBox * const 0x011fdef0, nsBoxLayoutState & {...}) line 1002
nsScrollBoxFrame::DoLayout(nsScrollBoxFrame * const 0x011fd39c, nsBoxLayoutState 
& {...}) line 377
nsBox::Layout(nsBox * const 0x011fd39c, nsBoxLayoutState & {...}) line 1002
nsContainerBox::LayoutChildAt(nsBoxLayoutState & {...}, nsIBox * 0x011fd39c, 
const nsRect & {x=0 y=0 width=9180 height=4470}) line 593 + 16 bytes
nsGfxScrollFrameInner::LayoutBox(nsBoxLayoutState & {...}, nsIBox * 0x011fd39c, 
const nsRect & {x=0 y=0 width=9180 height=4470}) line 1063 + 17 bytes
nsGfxScrollFrameInner::Layout(nsBoxLayoutState & {...}) line 1146
nsGfxScrollFrame::DoLayout(nsGfxScrollFrame * const 0x011fd2f8, nsBoxLayoutState 
& {...}) line 1071 + 15 bytes
nsBox::Layout(nsBox * const 0x011fd2f8, nsBoxLayoutState & {...}) line 1002
nsBoxFrame::Reflow(nsBoxFrame * const 0x011fd2c0, nsIPresContext * 0x02a51dc0, 
nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, unsigned int & 0) 
line 778
nsGfxScrollFrame::Reflow(nsGfxScrollFrame * const 0x011fd2c0, nsIPresContext * 
0x02a51dc0, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, 
unsigned int & 0) line 775 + 25 bytes
nsContainerFrame::ReflowChild(nsIFrame * 0x011fd2c0, nsIPresContext * 
0x02a51dc0, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, int 0, 
int 0, unsigned int 0, unsigned int & 0) line 693 + 31 bytes
ViewportFrame::Reflow(ViewportFrame * const 0x011fd248, nsIPresContext * 
0x02a51dc0, nsHTMLReflowMetrics & {...}, const nsHTMLReflowState & {...}, 
unsigned int & 0) line 546
nsHTMLReflowCommand::Dispatch(nsHTMLReflowCommand * const 0x0271f370, 
nsIPresContext * 0x02a51dc0, nsHTMLReflowMetrics & {...}, const nsSize & 
{width=9180 height=4470}, nsIRenderingContext & {...}) line 145
PresShell::ProcessReflowCommands(int 0) line 4239
PresShell::FlushPendingNotifications(PresShell * const 0x026cb490) line 3330
PresShell::EndReflowBatching(PresShell * const 0x026cb490, int 1) line 3353 + 15 
bytes
nsEditor::EndUpdateViewBatch() line 5219
nsEditor::EndPlaceHolderTransaction(nsEditor * const 0x026fade0) line 1245
nsAutoPlaceHolderBatch::~nsAutoPlaceHolderBatch() line 47 + 44 bytes
nsHTMLEditor::DeleteSelection(nsHTMLEditor * const 0x026fade0, 
nsIEditor::EDirection eNone) line 2354 + 30 bytes
nsHTMLEditorLog::DeleteSelection(nsHTMLEditorLog * const 0x026fade0, 
nsIEditor::EDirection eNone) line 159 + 13 bytes
nsGfxTextControlFrame2::SetTextControlFrameState(const nsString & {"1234"}) line 
2841
nsGfxTextControlFrame2::SetProperty(nsGfxTextControlFrame2 * const 0x011fe200, 
nsIPresContext * 0x02a51dc0, nsIAtom * 0x01364390 {"value"}, const nsString & 
{"1234"}) line 2163
nsGfxTextControlFrame2::RestoreState(nsGfxTextControlFrame2 * const 0x011fe208, 
nsIPresContext * 0x02a51dc0, nsIPresState * 0x0133c890) line 2986 + 34 bytes
FrameManager::RestoreFrameStateFor(FrameManager * const 0x026c9b40, 
nsIPresContext * 0x02a51dc0, nsIFrame * 0x011fe180, nsILayoutHistoryState * 
0x02a55080, nsIStatefulFrame::SpecialStateID eNoID) line 1595 + 25 bytes

Comment 9

[rubydoo123]

giving this to Kin to see if he can figure this one out, why is the update in 
the incorrect order?

Comment 10

[rods (gone)]

I wouldn't say it is in the incorrect order. I would say that it should cause a 
reflow everything the text is set, there needs to be a way to turn it off, 
because an initial reflow will be coming so it can then layout.

Comment 11

[kinmoz]

Accepting bug.

Comment 12

[leger]

beppe, this seems to work for us.  kcritzer out this next week, can you stil 
reproduce?

Comment 13

[rubydoo123]

this is easily reproducible on win98 using build from 2000082808

Comment 14

[kinmoz]

This should be a P1.

Comment 15

[Eric Pollmann]

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

Comment 16

[kinmoz]

Attachment: Test case with more TextWidgets.

Comment 17

[kinmoz]

Attachment: Patch that fixes the bug.

Comment 18

[kinmoz]

I just attatched a fix to this bug. I'm looking for a code reviewer. Any 
volunteers?

Comment 19

[kinmoz]

Fix checked into tip:

    mozilla/layout/html/forms/src/nsGfxTextControlFrame2.cpp  revision 1.90
    mozilla/layout/html/forms/src/nsGfxTextControlFrame2.h    revision 1.29

r=[e-mail]

Comment 20

[ckritzer (gone)]

The bug still exsits.  Sorry dude...
Reopening on:
- LinuxRH62 2000-09-13-08-M18 Commercial
- Win98     2000-09-13-08-M18 Mozilla
- MacOS86   2000-09-13-04-M18 Commercial

Comment 21

[ckritzer (gone)]

Attachment: bogus patch file upload test for Windows

Comment 22

[ckritzer (gone)]

Attachment: bogus patch file upload test for Linux

Comment 23

[ckritzer (gone)]

dang

I meant to upload these to bug#47552 (completely unrelated bug).  Please ignore!

Comment 24

[kinmoz]

Yeah, I put in some code to disable this fix, because I broke the IMAP password 
dialog. I have fix that I've been sitting on in my tree.

The funny thing is that on the day I disabled my fix, I could not reproduce the 
problem with the test case above ... even with my fix disabled, that's why I 
did not checkin the new fix. I guess the problem is back again.

Comment 25

[kinmoz]

Forgot to put [Fix in hand] in the status white board.

Comment 26

[kinmoz]

Checked in new fix:

    mozilla/layout/html/forms/src/nsGfxTextControlFrame2.cpp

      - Remove temporary call to SetInitialValue() in CreateAnonymousContent().
      - Move all calls to SetInitialValue() out from under the 'if' statement
        they were under. This will make sure that it gets called at least
        once since there are times when GetPrefSize() and Reflow() are not
        called during the initial reflow.

r=[e-mail]

Comment 27

[Vladimir Ermakov]

Dont see the problem. Verifying