Bug 25102 - TextMatches don't have a concept of active match
Summary: TextMatches don't have a concept of active match
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Finnur Thorarinsson
Depends on:
Reported: 2009-04-08 16:33 PDT by Finnur Thorarinsson
Modified: 2009-04-10 10:39 PDT (History)
1 user (show)

See Also:

Allowing the TextMatch marker to specify active/not (10.74 KB, patch)
2009-04-09 14:54 PDT, Finnur Thorarinsson
sullivan: review-
Details | Formatted Diff | Diff
Addressing review comments (10.76 KB, patch)
2009-04-09 15:46 PDT, Finnur Thorarinsson
sullivan: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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