[webkit-reviews] review requested: [Bug 40278] [EFL] EFLWebKit doesn't support viewport meta tag : [Attachment 58966] viewport-patch-for-eflwebkit-8

Thu Jun 17 00:16:10 PDT 2010

Gyuyoung Kim <gyuyoung.kim at samsung.com> has asked  for review:
Bug 40278: [EFL] EFLWebKit doesn't support viewport meta tag
https://bugs.webkit.org/show_bug.cgi?id=40278

Attachment 58966: viewport-patch-for-eflwebkit-8
https://bugs.webkit.org/attachment.cgi?id=58966&action=review

------- Additional Comments from Gyuyoung Kim <gyuyoung.kim at samsung.com>
Hi Lukas, 

Thank you for your detail review. I modified this patch according to your
points again.

>>I understood your point about using this function. However if you set the
viewport on this function it will trigger a re-layout. I think the double
signal you got on the other function is related to something else, since QT
guys are using it with no problem (afaik).

I tried to understand the viewport patch for QT port. As far as I understand,
the QT's patch sends signal regarding viewport in both cases there is viewport
and no viewport before any visual layout of the page. And, QT's patch prohibits
to set layout before finishing first layout. Because, to prevent re-layout.

void QWebPage::setPreferredContentsSize(const QSize& size) const 
{
    if (frame->d->initialLayoutComplete) // the initialLayoutComplete can be
set on FrameLoaderClientQt::dispatchDidFirstLayout()
	view->layout();

It means that application only can received data of viewport tag on callback
function for viewport. Application can't set layout with viewport data as soon
as viewport signal is received. Because, viewport tag should be located at
<head>...</head>. The first layout is not finished on <head>...</head>. Thus,
user needs to find good position in order to adjust viewport tag. It seems this
can prevent re-layout because application only can decide if viewport is adjust
or not. In other words, application is responsible for layout.

I modified this patch similar to QT port's patch and add an example code to
adjust viewport tag as below,

/**
 * This is en example function to adjust viewport via viewport tag's arguments.

 * Application can invoke this function in order to adjust viewport tag when it
is required.
 */
static void
viewport_set()
{
    ELauncher *app;
    app = (ELauncher*) eina_list_data_get(windows);

    ewk_view_fixed_layout_size_set(app->browser, app->viewport.w,
app->viewport.h);
    ewk_view_zoom_set(app->browser, app->viewport.initScale, 0, 0);
    if (!ewk_view_zoom_range_set(app->browser, app->viewport.minScale,
app->viewport.maxScale))
       info(" Fail to set zoom range. minScale = %f, maxScale = %f\n",
app->viewport.minScale, app->viewport.maxScale);
    ewk_view_user_scalable_set(app->browser, app->viewport.userScalable);
}

It seems to me that callback function regarding
dispatchDidFirstVisuallyNonEmptyLayout() is best. Of course, user can decide to
put the viewport_set().

I also prohibit to set layout before finishing first layout.

void ewk_view_fixed_layout_size_set(Evas_Object* o, Evas_Coord w, Evas_Coord h)

{
    EWK_VIEW_SD_GET_OR_RETURN(o, sd);
    EWK_VIEW_PRIV_GET_OR_RETURN(sd, priv);

    WebCore::FrameLoaderClientEfl* client =
static_cast<WebCore::FrameLoaderClientEfl*>(priv->main_frame->loader()->client(
));
    if (!client->getInitLayoutCompleted())
	return;
...

Thanks,
Gyuyoung Kim