[webkit-reviews] review denied: [Bug 25102] TextMatches don't have a concept of active match : [Attachment 29371] Allowing the TextMatch marker to specify active/not

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Apr 9 15:26:37 PDT 2009


John Sullivan <sullivan at apple.com> has denied Finnur Thorarinsson
<finnur.webkit at gmail.com>'s request for review:
Bug 25102: TextMatches don't have a concept of active match
https://bugs.webkit.org/show_bug.cgi?id=25102

Attachment 29371: Allowing the TextMatch marker to specify active/not
https://bugs.webkit.org/attachment.cgi?id=29371&action=review

------- Additional Comments from John Sullivan <sullivan at apple.com>
> 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.


More information about the webkit-reviews mailing list