The default bug view has changed. See this FAQ.

Findbar highlighting should not explicitly repaint selection

VERIFIED FIXED in mozilla1.9.1b1

Status

()

Toolkit
Find Toolbar
--
minor
VERIFIED FIXED
9 years ago
9 years ago

People

(Reporter: graememcc, Assigned: graememcc)

Tracking

({perf})

Trunk
mozilla1.9.1b1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

9 years ago
Created attachment 332268 [details] [diff] [review]
Patch v1

Following on from bug 429732 comment 14:

The highlighting methods are performing too much work by explicitly calling the selection controller's RepaintSelection method. The very act of adding a range to the selection will ensure it's contents will get repainted.

No tests provided due to trivial nature of patch.
Attachment #332268 - Flags: review?(mano)

Updated

9 years ago
Keywords: perf
(Assignee)

Updated

9 years ago
Target Milestone: mozilla1.9.1a2 → ---
Comment on attachment 332268 [details] [diff] [review]
Patch v1

r=mano
Attachment #332268 - Flags: review?(mano) → review+
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
Comment on attachment 332268 [details] [diff] [review]
Patch v1

>           if (aHighlight)
>             findSelection.addRange(aRange);
>           if (isEditable) {
>             if (!aHighlight)
>               findSelection.removeAllRanges();
>-            controller.repaintSelection(this.nsISelectionController.SELECTION_FIND);
>           }

How about this:

>           if (aHighlight)
>             findSelection.addRange(aRange);
>           else if (isEditable)
>             findSelection.removeAllRanges();
(Assignee)

Comment 3

9 years ago
Created attachment 337854 [details] [diff] [review]
Patch v2

Yes, much tidier!

This doesn't change the logic, so am assuming this doesn't require re-review.
Attachment #332268 - Attachment is obsolete: true
Attachment #337854 - Flags: review+
http://hg.mozilla.org/mozilla-central/rev/0478578dd7a1
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b1
No reported regression so far. Marking this bug as verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.