56156 - minimize and restore a scrolled window, always scrolled to top

Status: VERIFIED FIXED

(SeaMonkey :: UI Design, defect, P2)

Description

[buster]

1. WinNT build id 2000101008 (although I noticed it quite a while ago)
2. Open browser, load any large document (an lxr page will do)
3. scroll down near the bottom
4. minimize the window
5. restore the window
6. notice the page is scrolled to the top.  *very* annoying.


this happens in all apps:
7. open mail with a large inbox.  scroll down, minimize, restore, you're back at
the top
8. open IM, get a chat going, it auto-scrolls with new input.  minimize,
restore, you're back at the top.
9. etc.

Comment 1

[buster]

the more I use NS6, the more this costs me time and aggrevation.  IM sessions
are particularly annoying, because I minimize windows during idle times in the
conversation.

Comment 2

[buster]

I assigned this to eric only as a guess.  cc-ing trudelle in case he wants to
reassin, and to get his opinion on the severity.

Comment 3

[James Lariviere]

THis is a dup of bug 55033?

Comment 4

[buster]

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

Comment 5

[Syd Logan]

This does not appear to happen in branch build commercial on *Linux*, perhaps a
NT-only problem?

Comment 6

[buster]

don't know if this just depends on bug 46877, or if it is actually a duplicate.

Comment 7

[John Morrison]

This is a windows thing. See bug  45903 for some earlier discussion, although
I forget what the final thought was. hyatt?

Comment 8

[Eric Vaughan]

Attachment: Fix for crash

Comment 9

[John Morrison]

Um, Eric ... wrong bug for that attachment. Put it on bug 55909.

Comment 10

[Peter Trudelle]

This works fine for me in browser windows, using today's branch verification
build on Win98.  Is it only a problem on NT, or is it limited to other types of
windows?

Comment 11

[Hixie (not reading bugmail)]

It's definitely broken in Windows 2000.

Comment 12

[Peter Trudelle]

BTW, my opinion is that severity is probably normal or minor, since there is no
major loss of function exhibited here, and there is a workaround.  I'm not sure
it is the best use of Eric's time right now, but If a very simple/safe fix were
found in the next day or two, I'd hope it would not be too late to be considered
for N6.  adding helpwanted keyword., cc jrgm

Comment 13

[John Morrison]

I can reproduce this behaviour on windows98 with today's branch build.
Note, though, that this is not a new behaviour (as far as I recall);
see bug 45903.

Comment 14

[Eric Vaughan]

Please ignore the patch I attached earlier. Somehow I attached a patch to 
another bug to this one by accident.

Comment 15

[Hixie (not reading bugmail)]

FWIW, I do a lot of work with long documents (e.g. reading mail archives,
reading specifications) and if it wasn't for the fact that I *work* on Mozilla,
this bug would prevent me from using it. 

Comment 16

[Peter Trudelle]

That doesn't sound like typical usage; perhaps this is more of a dogfood issue?

Comment 17

[buster]

it's just one of these embarassing errors that, if easy to fix, would go a long
way towards making us look like a grown up, professional application.  It's the
kind of error you expect from beta-ware, not released software.  So, if it's
easy and low risk to fix, and it doesn't take long to fix, then it's worth a few
minutes and a rtm++.  but if any of these conditions aren't met, forget it for NS6.

Comment 18

[Eric Vaughan]

I believe this happens because when you minimize the window it gets reflowed 
very small on windows and then when it is maximized again it reflow back to the 
original size (Causing you to lose your scrollposition). So the bug is it should 
not reflow the window on a minimize. But I haven't investigated complelely so 
this is just a high level guess.

Comment 19

[Stephen Hassard]

perhaps this bug is related to <a
href="http://bugzilla.mozilla.org/show_bug.cgi?id=46877">bug 46877</a>?

Comment 20

[sairuh (rarely reading bugmail)]

winNT/w2k only...

Comment 21

[Eric Vaughan]

Ok I have confirmed that when a minimize the root window is sized to a very 
small size. So we reflow to that small size. Causing the scrollposition to be 
lost. This only happens on windows. The fix is on windows we should listen for a 
minimize event and not reflow the window to small. We could do this by notifying 
the window it has been minimized in some XP way and that is should not do and 
reflows. Off to Dan. 

Comment 22

[Eric Vaughan]

to dan

Comment 23

[Jim Roskind]

Discussed with PDT... and couldn't get support for a dogfood marking.
Marking dogfood-minus (even though I think this is pretty significant to usability)

Comment 24

[Peter Trudelle]

Does that mean PDT would not approve a simple/safe fix for N6?  cc jar for
answer. If this starts to look like wasted effort for N6, then we need to shift
to working on things that could matter.

Comment 25

[Jim Roskind]

I would *love* a simple and safe fix for this in N6.  Do you have one???

Comment 26

[Peter Trudelle]

not yet, but since it still might get in, it would be worth working on, so I'll
leave on rtm radar.

Comment 27

[David Hyatt]

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

Comment 28

[Dan M]

  Alright. I've actually taken time from my list of serious issues to investigate 

this bug. I've thrown together enough sample code to know that I can fix it with 

a small IDL change and beating lightly on maybe four source files. As far as I 

know it's a --snort-- safe enough change. I mean, it probably wouldn't hurt 

anything and doesn't seem to in my testing, but it is layout, after all.

  So tell me, should I spend the time to code it up?

Comment 29

[Peter Trudelle]

That depends on what you mean by 'lightly beating'. If it is only a few lines of
code, then certainly.  If it is a few pages, then maybe not.  Sounds to me like
it is worth trying, and at the very least the trunk will benefit.

Comment 30

[Dan M]

  It turns out the knowledge that a window has been minimized is available -- 
it's nsIWidget::GetSizeMode. It would be quite straightforward to disable 
nsPresShell::ResizeReflow if its container's parent widget claims to be 
minimized, which is my reading of what Eric suggests we do above. (Note that it's 
a bad/old interface that would be handier if moved into nsIDOMWindowInternal, but 
what the heck. It's there. You could use it.)
  However, conversations with av, buster and nisheeth make me want to not turn 
off resizereflow. They seem to think it might not be harmful to build up a queue 
of deferred reflows while minimized, and trot them out for processing once 
restored. But simply skipping them sounds dangerous.
  The fact that I've never looked into the scrollframe code strangely doesn't 
prevent my suggesting that the right fix for this problem is for them to just 
deal with being overshrunk, and return themselves to their last position once 
resized to something reasonable.
  Regardless of that plan's feasibility, the minimization information Eric wanted 
is available. Back to him.

Comment 31

[Mike Pinkerton (not reading bugmail)]

taking this over

Comment 32

[Eric Vaughan]

I think the real fix here is this:

Windows: we get a size event to size the window to a very small size when the 
window is minimized.

Mac: We get no size event

Linux: We get no size event

The right fix is platform parity. The windows specific code should suppress this 
size notification to the presshell if it is being minimized.

Comment 33

[Mike Pinkerton (not reading bugmail)]

here's the patch. can someone review? (cc'ing rods). Note, this isn't the most
perfectly optimal patch, just the one with the fewest number of lines changed
(to appease the PDT gods). I won't use a |break| on the trunk.

Index: nsWindow.cpp
===================================================================
RCS file: /cvsroot/mozilla/widget/src/windows/nsWindow.cpp,v
retrieving revision 3.296
diff -r3.296 nsWindow.cpp
3165a3166,3173
>
>               // If we're being minimized, don't send the resize event to
Gecko because
>               // it will cause the scrollbar in the content area to go away
and we'll
>               // forget the scroll position of the page.
>               if ( !newWidth && !newHeight ) {
>                 result = PR_FALSE;
>                 break;
>               }

Comment 34

[rods (gone)]

I tested it out - It looks good enough    

r=rods

Comment 35

[David Hyatt]

a=hyatt

Comment 36

[selmer (gone)]

This bug is in candidate limbo.  We will reconsider this fix once we have a 
candidate in hand, but we can't take this fix before then.

Comment 37

[selmer (gone)]

We are moving toward the candidate.  Please check this fix into the trunk so we 
can get get some cook time.

Comment 38

[Mike Pinkerton (not reading bugmail)]

landed on trunk.

Comment 39

[R.K.Aa.]

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

Comment 40

[dales]

I was happy with the WinNT performance from yesterday's build; but
now I'm running the 2000103004 (Mtrunk) build and the loss of scroll
position bug is back.  Seems like something regressed ...

	Dale
 

Comment 41

[Gilles Durys]

It works with 2000103004 on win98

Comment 42

[Mike Pinkerton (not reading bugmail)]

Just tried this with the 10/30/00-9am trunk on win2k and it works just dandy.

Comment 43

[Phil Peterson]

PDT marking [rtm++]. This bug is now out of limbo and approved for checkin to
the branch. Please check in ASAP.

Comment 44

[Mike Pinkerton (not reading bugmail)]

landed on branch

Comment 45

[dales]

OK when I saw others showing success, I recalled that I use WindowBlinds on this
NT box.  I unloaded it, and I agree ... scroll position is retained.

Then I reloaded WindowBlinds, and scroll position is lost again.  It another
WindowBlinds compatibility problem ... but at least there's the workaround
"Unload WondowBlinds" pending time for a proper fix.

	Dale

Comment 46

[nhottanscp]

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

Comment 47

[Peter Trudelle]

Verifying fixed on Win98, using 10-31-14 MN6 candidate build.  Good work!