[Webkit-unassigned] [Bug 79820] [BlackBerry] Implement features for find-in-page

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=79820





--- Comment #15 from Andy Chen <andchen at rim.com>  2012-02-29 07:26:37 PST ---
(In reply to comment #10)
> (In reply to comment #9)
> > (From update of attachment 129335 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=129335&action=review
> 

> > 
> > > Source/WebKit/blackberry/WebKitSupport/InPageSearchManager.cpp:108
> > > +        m_currentActiveMatchFrame = DOMSupport::incrementFrame(m_currentActiveMatchFrame, forward, true);
> > > +        if (findAndMarkText(text, m_activeMatch.get(), findOptions))
> > > +            return true;
> > > +    } while (m_currentActiveMatchFrame && startFrame != m_currentActiveMatchFrame);
> > 
> > I think it could be "better" here to actually pass the Frame* you want to search text at, instead of the setting a class member m_currentActiveMatchFrame, and use it from findAndMarkText. 
> > 
> > That is how I would do it, but if you feel strong about how it is now, that is fine with me.
> > 
> 
> Would that allow the removal of m_currentActiveMatchFrame entirely?  If so that is preferred.
> 

(In reply to comment #10)
> (In reply to comment #9)
> > (From update of attachment 129335 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=129335&action=review

> > 
> > > Source/WebKit/blackberry/WebKitSupport/InPageSearchManager.cpp:108
> > > +        m_currentActiveMatchFrame = DOMSupport::incrementFrame(m_currentActiveMatchFrame, forward, true);
> > > +        if (findAndMarkText(text, m_activeMatch.get(), findOptions))
> > > +            return true;
> > > +    } while (m_currentActiveMatchFrame && startFrame != m_currentActiveMatchFrame);
> > 
> > I think it could be "better" here to actually pass the Frame* you want to search text at, instead of the setting a class member m_currentActiveMatchFrame, and use it from findAndMarkText. 
> > 
> > That is how I would do it, but if you feel strong about how it is now, that is fine with me.
> > 
> 
> Would that allow the removal of m_currentActiveMatchFrame entirely?  If so that is preferred.
> 

I don't think m_currentActiveMatchFrame could be removed entirely. For this function, it could be called by passing in the pointer.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.