311304 - Selection is not erased (turds) when Find Next from the Find window causes the page to scroll

Status: VERIFIED FIXED

(Camino Graveyard :: Page Layout, defect, P2)

Description

[Simon Fraser [no longer active]]

We can get selection turds when a Find Next from the Find window causes the page
to scroll.

Steps:
1. Load http://www.faqs.org/rfcs/rfc2639.html
2. Bring up the Find window (Command-F)
3. Enter "client-error-request-value-too-long" (without quotes)
4. Keep clicking the "Find Next" button.

Note that after a couple of times, you'll see it selected more than once.
Resizing the window causes the page to redraw and fix the drawing turds.

This only seems to happen if doing the Find Next from the Find window. Maybe it
only happens if the content area is not focussed.

Comment 1

[Simon Fraser [no longer active]]

Happens in 1.0a1 and latest branch. Have not tested trunk.

Comment 2

[Mike Pinkerton (not reading bugmail)]

didn't we already fix this in the 0.7 timeframe? any idea of a regression window?

Comment 3

[Simon Fraser [no longer active]]

Looks like regression from bug 294415 (regressed on 6/16).
We probably invalidate after scrolling, so the inval is in the wrong location.

Comment 4

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Since this is a regression, it would be nice to fix for 1.1

Comment 5

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Stupid return key :/  (and this should be everywhere, now, since it was before 1.8 branched)

Comment 6

[Stuart Morgan]

Attachment: Tracks pending invalidation rects

This puts any pending invalidation rects into a queue, and makes sure that they are updated to reflect any scrolling that happens between their creation and actually triggering a redraw.  There's still a slight flicker of the old selection in the test case, but I don't see any way around that given the pending drawing scheme.

Simon, do you have time to take a look at this?

Comment 7

[Stuart Morgan]

This is a trunk path, by the way; it probably won't apply to branch as-is given the recent Cocoa widgets work.

Comment 8

[Simon Fraser [no longer active]]

Comment on attachment 243134 [details] [diff] [review]
Tracks pending invalidation rects

Ooh, does this fix scrollbar turds too (bug 301152)?

Comment 9

[Stuart Morgan]

Comment on attachment 243134 [details] [diff] [review]
Tracks pending invalidation rects

(In reply to comment #8)
> Ooh, does this fix scrollbar turds too (bug 301152)?

I wondered that myself, but I can't find a reproducible test case.  I guess we'll wait and see.

Comment 10

[Josh Aas]

Comment on attachment 243134 [details] [diff] [review]
Tracks pending invalidation rects

I can't sr code outside of the camino directory. But this looks good to me.

Comment 11

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 243134 [details] [diff] [review]
Tracks pending invalidation rects

sr=pink

Comment 12

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Stuart, are you going to do a branch version of this (and then we can start begging for branch approval, since it's in Core code), or are we not going to fix this on the branch?

Comment 13

[Josh Aas]

landed on trunk

Comment 14

[Stuart Morgan]

Attachment: branch version

I am indeed planning to land this on the branch; I just need to find out if we need any kind of approval at this point.

Comment 15

[Stuart Morgan]

I should probably mention that on branch this is Camino-only, since we're the only branch cocoa widget client.

Comment 16

[Maximilian Ruderburg [:max] (Arctic Revolution)]

Since this landed on the trunk, resolving this bug as fixed. It'll also help drivers when they go through our bug.

Comment 17

[Maximilian Ruderburg [:max] (Arctic Revolution)]

Verified on trunk using Version 2006112004 (1.2+).

Comment 18

[Daniel Veditz [:dveditz]]

Comment on attachment 245103 [details] [diff] [review]
branch version

approved for 1.8 branch, a=dveditz for drivers

Comment 19

[Stuart Morgan]

Checked in on MOZILLA_1_8_BRANCH.