Bug 49275 - Need WebKit2 mechanism for getting the visible page text
Summary: Need WebKit2 mechanism for getting the visible page text
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
Keywords: InRadar
Depends on:
Reported: 2010-11-09 13:38 PST by Sam Weinig
Modified: 2010-11-09 14:27 PST (History)
0 users

See Also:

Patch (20.77 KB, patch)
2010-11-09 13:46 PST, Sam Weinig
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 2010-11-09 13:38:35 PST
Need WebKit2 mechanism for getting the visible page text
Comment 1 Sam Weinig 2010-11-09 13:38:58 PST
Comment 2 Sam Weinig 2010-11-09 13:46:45 PST
Created attachment 73408 [details]
Comment 3 Darin Adler 2010-11-09 13:54:12 PST
Comment on attachment 73408 [details]

View in context: https://bugs.webkit.org/attachment.cgi?id=73408&action=review

> WebKit2/UIProcess/WebPageProxy.cpp:74
> +void invalidateCallbackMap(HashMap<uint64_t, RefPtr<T> >& map)

Could just use T in this template in the places you use RefPtr<T>. Would read nicer and work just as well.

> WebKit2/UIProcess/API/C/WKPage.cpp:333
> +void WKPageGetContentsAsString(WKPageRef pageRef, void *context, WKPageGetContentsAsStringFunction callback)

Should be void* instead of void *.

> WebKit2/WebProcess/WebPage/WebFrame.cpp:248
> +            WebFrame* webFrame = static_cast<WebFrameLoaderClient*>(child->loader()->client())->webFrame();
> +            builder.append(webFrame->contentsAsString());

Not sure this needs a local variable for webFrame.

> WebKit2/WebProcess/WebPage/WebFrame.cpp:250
> +            if (child->tree()->nextSibling())
> +                builder.append(' ');

I would suggest:

    if (!builder.isEmpty())
        builder.append(' ');

at the start of the loop instead of this code at the end of the loop.

> WebKit2/WebProcess/WebPage/WebFrame.cpp:254
> +        
> +        // FIXME: It may make sense to use toStringPreserveCapacity() here.
> +        return builder.toString();

The blank line here makes the paragraphing a little strange.
Comment 4 Sam Weinig 2010-11-09 14:27:25 PST
Landed in 71678.