Bug 25102

Summary: TextMatches don't have a concept of active match
Product: WebKit Reporter: Finnur Thorarinsson <finnur.webkit>
Component: New BugsAssignee: Finnur Thorarinsson <finnur.webkit>
Status: RESOLVED FIXED    
Severity: Normal CC: sullivan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Allowing the TextMatch marker to specify active/not
sullivan: review-
Addressing review comments sullivan: review+

Description Finnur Thorarinsson 2009-04-08 16:33:29 PDT
John Sullivan recommended I file a bug to track this.

The TextMatches in WebKit don't differentiate between active and inactive matches. It would be very useful for the ports that use TextMatches for FindInPage to be able to specify which TextMatches are active and which are not. This would also involve splitting up platformTextSearchHighlightColor into two, to provide an Active color and an Inactive color.

I'm in the middle of writing such a patch. This bug tracks that effort.
Comment 1 Finnur Thorarinsson 2009-04-09 14:54:38 PDT
Created attachment 29371 [details]
Allowing the TextMatch marker to specify active/not

Attached is a proposed patch that fixes this.
Comment 2 John Sullivan 2009-04-09 15:26:36 PDT
Comment on attachment 29371 [details]
Allowing the TextMatch marker to specify active/not

> Index: dom/Document.h
> ===================================================================
> --- dom/Document.h	(revision 42365)
> +++ dom/Document.h	(working copy)
> @@ -699,6 +699,8 @@ public:
>      void setRenderedRectForMarker(Node*, DocumentMarker, const IntRect&);
>      void invalidateRenderedRectsForMarkersInRect(const IntRect&);
>      void shiftMarkers(Node*, unsigned startOffset, int delta, DocumentMarker::MarkerType = DocumentMarker::AllMarkers);
> +    void setMarkersActive(Range*, bool);
> +    void setMarkersActive(Node*, unsigned, unsigned, bool);

The two unsigned parameters here should be given names in this declaration since it's otherwise not clear what they represent.

The patch otherwise seems fine to me. Please submit a new patch with this change.
Comment 3 Finnur Thorarinsson 2009-04-09 15:46:29 PDT
Created attachment 29377 [details]
Addressing review comments

Reviewed in half an hour. Not a bad turnaround. Not at all. :)
Comment 4 Darin Fisher (:fishd, Google) 2009-04-10 10:25:25 PDT
Landed as: http://trac.webkit.org/changeset/42393
Comment 5 Darin Fisher (:fishd, Google) 2009-04-10 10:39:45 PDT
And the follow-up bustage fix:  http://trac.webkit.org/changeset/42394