[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 #10 from Mike Fenton <mifenton at rim.com>  2012-02-29 07:07:36 PST ---
(In reply to comment #9)
> (From update of attachment 129335 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=129335&action=review

> > Source/WebKit/blackberry/WebKitSupport/InPageSearchManager.cpp:95
> > +        | StartInSelection;
> 
> do you need the StartInSelection bit, if you just cleared any possible selection in the block above?

Correct, it's only needed in the cases with a selection.  No real harm in setting it though if the Range provided is empty.

> 
> > 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.

> > Source/WebKit/blackberry/WebKitSupport/InPageSearchManager.cpp:125
> > +        && m_activeSearchString == text.substring(0, m_activeSearchString.length()))
> 
> prefix only? or sufix or a substring in general?
> 

Prefix only, this case is when adding an additional character to a non-matched string.  No match is possible.

> > Source/WebKit/blackberry/WebKitSupport/InPageSearchManager.cpp:164
> > +        if (frame == m_webPage->mainFrame()) // Don't need to unmark and notify client because the page will be destroyed.
> 
> if it is the mainframe, do we need to do any of the above anyways?

Yes, we aren't unloading the instance so on page navigation we need to clear the cached objects.

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