141249 - OS9 scrolling doesn't repaint correctly in outliner or in other places

Status: VERIFIED FIXED

(Core :: XUL, defect)

Description

[Mike Pinkerton (not reading bugmail)]

After smfr's scrolling speedups, scrolling with a scrollwheel in outliner would 
mess up. This didn't impact osx because we use the OS call to do all our 
scrolling.

The problem was that simon put in a call to OffsetRgn, but this was already taken 
care of by Begin/EndUpdate so the resulting region that got invalidated was 
incorrect. 

Patch upcoming.

Comment 1

[Mike Pinkerton (not reading bugmail)]

Attachment: fix invalidated region

fixes bad invalidates on os9

needs r/sr

Comment 2

[Mike Pinkerton (not reading bugmail)]

this needs to go on the trunk and the branch. cc'ing some more mac guys.

Comment 3

[Steve Dagley]

Comment on attachment 81703 [details] [diff] [review]
fix invalidated region

r=sdagley

Comment 4

[Conrad Carlen (not reading bugmail)]

Does the case which Simon fixed, scrolling a dirty region (happens with "Find
Next" in a doc which contains several close-together occurances) still work?

Comment 5

[Mike Pinkerton (not reading bugmail)]

to answer conrad's question about the closely located find scrolling turds, it
appears that smfr's last patch also regressed that fix, and this patch fixes
those as well. Note that it doesn't fix every problem with find scrolling, but
it doesn't appear to intoduce any new ones.

Comment 6

[Mike Pinkerton (not reading bugmail)]

Attachment: better fix

ok, so it appears that simon was in fact incorrect when he adjusted the visrgn
into "local" coordinates of the widget. it caused not only the scrollwheel
regression but turds appearing when scrolling small amounts and the find window
would get blitted into the content area.

this patch removes both instances of offsetting the region and fixes both
regressions. when verifying this bug, please make sure that 127362 is still
fixed (scrolling speed on long pages) and whatever bug the "scrolling small
increments while doing a find leaves turds" is.

need r/sr and if anyone in QA can run with this patch, it would be appreciated

Comment 7

[Mike Pinkerton (not reading bugmail)]

cc'ing brade as we discussed so she can run with this patch tomorrow.

Comment 8

[Patrick C. Beard]

Comment on attachment 81765 [details] [diff] [review]
better fix

sr=beard

Comment 9

[Steve Dagley]

Comment on attachment 81765 [details] [diff] [review]
better fix

r=sdagley

Comment 10

[Asa Dotzler [:asa]]

Comment on attachment 81765 [details] [diff] [review]
better fix

a=asa (on behalf of drivers) for checkin to the 1.0 branch

Comment 11

[Mike Pinkerton (not reading bugmail)]

oh mother, where art thou?

Comment 12

[Mike Pinkerton (not reading bugmail)]

landed on the trunk. 

jrgm, can you work with se to verify that 127362 is still fixed even with this
partial backout? and also give this a strong workout on os9? we need to make
sure this is good and true before we put it on the branch.

Comment 13

[John Morrison]

I'll give this a workout on os9 and sairuh has graciously offered to recheck 
that bug 127362 has not regressed, in the next available trunk mac builds.

Comment 14

[Jaime Rodriguez, Jr.]

Good job on the fix, but we think this one can wait till RTM. adt1.0.0-/[adt3]

Comment 15

[Kathleen :Brade]

please, please, PLEASE reconsider--this bug is absolutely horrible!  It's at
least an adt2 in my experience but at times it's a show stopper (I will not be
able to use the beta without this fix; I'll have to stick to trunk builds).

I am seeing lots of repaint issues; I will attach some screenshots.  The worst
problem is in my inbox.  When I delete a message it incorrectly updates all of
the messages listed below it and sometimes I have deleted the wrong messages
because it wasn't rendered properly.

Oh, this has nothing to do with "scrollwheel" really; it's a general updating issue.

Comment 16

[Mike Pinkerton (not reading bugmail)]

i agree with brade. this is a STOP SHIP regression.

Comment 17

[Asa Dotzler [:asa]]

This is stopship for mozilla1.0 RC2 (that's what the 138000 dependency means).
We must get this in before tomorrow.

Comment 18

[Kathleen :Brade]

upping back to adt1; this is such a showstopper; it is ***SOOOOooooo***
embarrassing to ship a beta with all of these update problems.

initial jpeg coming...

Comment 19

[Kathleen :Brade]

Attachment: 2 examples of bad layout/rendering

Comment 20

[Conrad Carlen (not reading bugmail)]

> upping back to adt1; this is such a showstopper; it is ***SOOOOooooo***
embarrassing

I completely agree. Given the severity of the problem, the risk posed by the
patch would have to be so huge (like deleting your whole mailbox) to justify
thinking we don't need this fix in the beta. I'm confident that there's little
to no risk here.

Comment 21

[sairuh (rarely reading bugmail)]

tested using 2002.05.02.03-trunk bits on Mac OS 10.1.4, with the classic (aqua)
theme. the scrollwheel behaves fine --i checked out the following areas:

webpages
bookmarks mgr
download mgr
thread and folder panes of mailnews 3pane window
history window
xul file:/// listing

gonna check this out on mac os 9.1 soon...

btw, does it matter what theme i'm using? should i do both?

Comment 22

[sairuh (rarely reading bugmail)]

looks good on 9.1 (2002.05.02.03-trunk), both themes. both areas with outliners
and web pages scroll fine with the scrollwheel (same tests in comment 21).

Comment 23

[Jaime Rodriguez, Jr.]

adt1.0.0+ (on ADT's behalf) approval for checkin to the 1.0 branch. Pls check
this in to 1.0 branch today, then add the fixed1.0.0 keyword.

Comment 24

[Mike Pinkerton (not reading bugmail)]

landed on branch

Comment 25

[sairuh (rarely reading bugmail)]

looks fine (no regressions) using 2002.05.03.09-1.0.0 bits on Mac 10.1.4.

still waiting on 9.x branch bits to become available...

Comment 26

[sairuh (rarely reading bugmail)]

vrfy'd fixed using 2002.05.06.05-1.0.0 (branch) comm bits on mac 9.1.