563416 - textarea.clientWidth reports different results on Windows after Undo Close Tab and on initial load in this testcase

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Nickolay_Ponomarev]

Attachment: testcase

From bug 542375. I noticed that if I modify the testcase in that bug to alert all results at the end of the script (updated testcase attached), windows trunk alerts wrong values when doing Undo Close Tab, but not when loading the testcase directly.

Steps to reproduce:
1. Open the attached testcase (dismiss the alert, which should say 179,200,200,200)
2. Open a new tab
3. Close the original tab with the testcase
4. Undo Close Tab

Actual results: 179,179,179,179 alerted
Expected: same as when loaded initially.

Comment 1

[Nickolay_Ponomarev]

Attachment: frame dump

bz requested a "a frame dump from after that undo close tab", so here's what I did:
1. reproduced the bug with steps from comment 0 on a debug build from 41743:02ca6f9215bc
2. Attached to the process while it was displaying the alert with the incorrect values
3. Switched to the nsGlobalWindow::Alert frame
4. Ran this:
nsFrame::DumpFrameTree(((nsIPresShell*)this->mDoc.mRawPtr->mPresShell)->GetRootFrame())

Results are attached.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Ok, so here's the relevant part of the frame dump:

nsTextControlFrame@09331318 {0,60,12360,360} [state=80c04010] [content=06AD0D10] [sc=092E2670]<
  HTMLScroll(div)(-1)@092E2C20 next=092E3E78 {180,180,10740,0} [state=00084010] [content=08ECFC38] [sc=092E3998]<
 

12360/60 == 206.  10740/60.0 == 179.

roc, dbaron, any idea why the scrollframe doesn't end up the right width in this case?  Something about stack layout optimizing away reflows of kids when it shouldn't, maybe?  And in particular, if the text control reflow root layout happens before the viewport one, the scrollframe will have no dirty kids, and we might optimize away reflowing it on resize of the text input, if things are buggy somewhere.

Comment 3

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

This fixes it for me on trunk (Linux).  The problem is that 
nsRect::operator== considers the rects equal since the height is zero.

Comment 4

[Boris Zbarsky [:bzbarsky]]

I'll bet money this fixes bug 542375 too.

Comment 5

[Mats Palmgren (inactive)]

Nickolay, can you confirm the patch fixes it for you?
I've made 1.9.2 TryServer builds with the fix if you need them:
https://build.mozilla.org/tryserver-builds/[e-mail]-563416-192/

I can't reproduce the STR in this bug on Linux trunk.  However, I do see
the problem when Reloading the testcase (not Shift+Reload).

Comment 6

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1, with mochitest

Comment 7

[Nickolay_Ponomarev]

Yeah, on 1.9.2 it fixes both this bug as reported and bug 542375.

Does anybody else wonder why Mac is different here? The textarea there is 13px high according to the DOMi and it's visually higher than on Windows too.

Comment 8

[Boris Zbarsky [:bzbarsky]]

The height difference is just a matter of native theming.

The other could be a matter of when the viewport reflow is scheduled compared to when the value change happens.  Or something about height changing that causes the stack to deal correctly to start with.  Or who knows...

Comment 9

[Mats Palmgren (inactive)]

http://hg.mozilla.org/mozilla-central/rev/e26e8284484b
http://hg.mozilla.org/mozilla-central/rev/cee251f569c5

I disabled the test due to Tinderbox orange on OSX, I'm investigating
the test failure locally...

Comment 10

[Mats Palmgren (inactive)]

Fixed the test:
http://hg.mozilla.org/mozilla-central/rev/9f12b80e4490