[Webkit-unassigned] [Bug 103886] [WK2][WTR] Enable view mode media feature layout test

[bugzilla-daemon at webkit.org]

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





--- Comment #4 from Christophe Dumez <christophe.dumez at intel.com>  2012-12-05 03:17:26 PST ---
(From update of attachment 177710)
View in context: https://bugs.webkit.org/attachment.cgi?id=177710&action=review

> LayoutTests/ChangeLog:8
> +        Unskipping the following tests:

... for EFL WebKit2.

> Source/WebCore/ChangeLog:8
> +        Export WebCore::Page::setViewMode method to WebKit(2).

"Export symbol for WebCore::Page::setViewMode on Mac." ?

> Source/WebCore/ChangeLog:10
> +        No new tests (OOPS!).

You should indicate something like:
"No new tests, no behavior change for layout tests."

> Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp:456
> +    WebCore::Page* page = toImpl(pageRef)->corePage();

It is uncommon to access the WebCore page from WKBundlePage I believe. I would usually add a new method to WebPage and call that one from WKBundlePage. WKBundlePage is usually just a C wrapper around WebPage API.

> Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePage.cpp:468
> +        page->setViewMode(WebCore::Page::ViewModeMinimized);

Maybe an ASSERT_NOT_REACHED(); at this end of this function just in case?

> Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePage.h:398
> +WK_EXPORT void WKBundlePageSetViewMode(WKBundlePageRef pageRef, WKStringRef mode);

Unless there is a specific need to this to be public, it should probably go to WKBundlePagePrivate.h. At least so far it is used by WKTR only.

> Tools/ChangeLog:3
> +        [WK2][WTR] Enable view mode media feature layout test

"Implement testRunner.setViewModeMediaFeature() in WebKitTestRunner" may be a better title.

> Tools/ChangeLog:8
> +        InjectedBundle API extended to set view mode media feature.

"Implement testRunner.setViewModeMediaFeature() in WebKitTestRunner." ?

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