Summary: | [WK2] Replace some internal API usage in EwkView with C API | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kenneth Rohde Christiansen <kenneth> | ||||
Component: | WebKit2 | Assignee: | Kenneth Rohde Christiansen <kenneth> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | benjamin, d-r, gyuyoung.kim, mikhail.pozdnyakov, rakuco, tmpsantos, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 107657 | ||||||
Attachments: |
|
Description
Kenneth Rohde Christiansen
2013-01-23 02:10:38 PST
Created attachment 184183 [details]
Patch
Comment on attachment 184183 [details]
Patch
LGTM.
Comment on attachment 184183 [details]
Patch
Looks fine.
Comment on attachment 184183 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=184183&action=review Really nice. Just one API thingy you might want to look into: > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:758 > - if (unreachableUrl && *unreachableUrl) > - impl->page()->loadAlternateHTMLString(String::fromUTF8(html), baseUrl ? String::fromUTF8(baseUrl) : "", String::fromUTF8(unreachableUrl)); > - else > - impl->page()->loadHTMLString(String::fromUTF8(html), baseUrl ? String::fromUTF8(baseUrl) : ""); > + WKRetainPtr<WKStringRef> wkHTMLString = adoptWK(WKStringCreateWithUTF8CString(html)); > + WKRetainPtr<WKURLRef> wkBaseURL = adoptWK(WKURLCreateWithUTF8CString(baseUrl)); > + > + if (unreachableUrl && *unreachableUrl) { > + WKRetainPtr<WKURLRef> wkUnreachableURL = adoptWK(WKURLCreateWithUTF8CString(unreachableUrl)); > + WKPageLoadAlternateHTMLString(impl->wkPage(), wkHTMLString.get(), wkBaseURL.get(), wkUnreachableURL.get()); > + } else > + WKPageLoadHTMLString(impl->wkPage(), wkHTMLString.get(), wkBaseURL.get()); The new code is nicer than the old one. Comment on attachment 184183 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=184183&action=review > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:568 > - impl->page()->loadURL(url); > + WKRetainPtr<WKURLRef> wkUrl = adoptWK(WKURLCreateWithUTF8CString(url)); > + WKPageLoadURL(impl->wkPage(), wkUrl.get()); This change the behavior. You know supports UTF8 instead of Latin1. You may want to update the documentation and tests. Created #107739 to track the unit test and documentation update. Comment on attachment 184183 [details] Patch Clearing flags on attachment: 184183 Committed r140600: <http://trac.webkit.org/changeset/140600> All reviewed patches have been landed. Closing bug. |