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

VERIFIED FIXED in Camino1.5

Status

P2
minor
VERIFIED FIXED
13 years ago
12 years ago

People

(Reporter: sfraser_bugs, Assigned: stuart.morgan+bugzilla)

Tracking

({fixed1.8.1.1})

unspecified
Camino1.5
PowerPC
Mac OS X
fixed1.8.1.1
Bug Flags:
camino1.0 -

Details

(URL)

Attachments

(2 attachments)

(Reporter)

Description

13 years ago
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.
(Reporter)

Comment 1

13 years ago
Happens in 1.0a1 and latest branch. Have not tested trunk.
Priority: -- → P2
Target Milestone: --- → Camino1.0
didn't we already fix this in the 0.7 timeframe? any idea of a regression window?
(Reporter)

Comment 3

13 years ago
Looks like regression from bug 294415 (regressed on 6/16).
We probably invalidate after scrolling, so the inval is in the wrong location.
Depends on: 294415
Keywords: regression
Flags: camino1.0?
Severity: normal → minor
Flags: camino1.0? → camino1.0-
Target Milestone: Camino1.0 → Camino1.1
Since this is a regression, it would be nice to fix for 1.1
Summary: Selection is not erased when Find Next from the Find window causes the page to scroll → Selection is not erased (turds) when Find Next from the Find window causes the page to scroll
Stupid return key :/  (and this should be everywhere, now, since it was before 1.8 branched)
Assignee: mikepinkerton → nobody
QA Contact: page.layout
Version: Trunk → unspecified
(Assignee)

Comment 6

12 years ago
Created attachment 243134 [details] [diff] [review]
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?
Assignee: nobody → stuart.morgan
Status: NEW → ASSIGNED
Attachment #243134 - Flags: review?(sfraser_bugs)
(Assignee)

Comment 7

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

Comment 8

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

Ooh, does this fix scrollbar turds too (bug 301152)?
Attachment #243134 - Flags: review?(sfraser_bugs) → review+
(Assignee)

Comment 9

12 years ago
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.
Attachment #243134 - Flags: superreview?(mikepinkerton)
(Assignee)

Updated

12 years ago
Attachment #243134 - Flags: superreview?(mikepinkerton) → superreview?(joshmoz)

Comment 10

12 years ago
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.
Attachment #243134 - Flags: superreview?(joshmoz) → superreview?(mikepinkerton)
Comment on attachment 243134 [details] [diff] [review]
Tracks pending invalidation rects

sr=pink
Attachment #243134 - Flags: superreview?(mikepinkerton) → superreview+
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

12 years ago
landed on trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(Assignee)

Comment 14

12 years ago
Created attachment 245103 [details] [diff] [review]
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.
(Assignee)

Updated

12 years ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Updated

12 years ago
Attachment #245103 - Flags: approval1.8.1.1?
(Assignee)

Comment 15

12 years ago
I should probably mention that on branch this is Camino-only, since we're the only branch cocoa widget client.
Since this landed on the trunk, resolving this bug as fixed. It'll also help drivers when they go through our bug.
Status: REOPENED → RESOLVED
Last Resolved: 12 years ago12 years ago
Resolution: --- → FIXED
Verified on trunk using Version 2006112004 (1.2+).
Status: RESOLVED → VERIFIED
Comment on attachment 245103 [details] [diff] [review]
branch version

approved for 1.8 branch, a=dveditz for drivers
Attachment #245103 - Flags: approval1.8.1.1? → approval1.8.1.1+
(Assignee)

Comment 19

12 years ago
Checked in on MOZILLA_1_8_BRANCH.
Keywords: regression → fixed1.8.1.1
You need to log in before you can comment on or make changes to this bug.