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

[bugzilla-daemon at webkit.org]

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 58643:  viewport-patch-for-eflwebkit-6
https://bugs.webkit.org/attachment.cgi?id=58643&action=review

------- Additional Comments from Gyuyoung Kim <gyuyoung.kim at samsung.com>
I tried to make a patch accroding to Gustavo and Jesus Sanchez-Palencia's
advice. 

First, I distinguish zoom_range variables from viewport struct in order to set
zoom_range by application or users.

+	 struct {
+	     float min_scale;
+	     float max_scale;
+	 } viewport;
+	 struct {
+	     float min_scale;
+	     float max_scale;
+	 } zoom_range;

So, zoom range can be changed by ewk_view_zoom_range_set() function.

Second, I modify this patch to send signal(viewport,changed) both there is
viewport and no viewport. So, if website contains "viewport" meta tag, viewport
signal is sent by didReceiveViewportArguments() in ChromeClientEfl.cpp.
However, if there no viewport, the signal(viewport,changed) is sent by
dispatchDidFirstVisuallyNonEmptyLayout() in FrameLoaderClientEfl.cpp. As far as
i know, the dispatchDidFirstVisuallyNonEmptyLayout() will be invoked when
layout is just started(). Thus, I think viewport signal need to be sent by the
function when there are no viewport meta tag.

If the signal already was sent by viewport meta tag, we should not send signal
duplicately. So, I add a condition to check if layout is done or not as below,

void FrameLoaderClientEfl::dispatchDidFirstVisuallyNonEmptyLayout()
{
-    notImplemented();
+    if (!ewk_view_init_layout_completed_get(m_view)) {
+	 ViewportArguments arguments;
+	 ewk_view_viewport_set(m_view, (int)arguments.width,
(int)arguments.height, arguments.initialScale, arguments.minimumScale,
arguments.maximumScale, arguments.userScalable);
+    }
}

If website doesn't contain viewport meta tag, all data except for userScalable
is set as "-1". So, application need to process them when viewport's datas are
"-1". I mentioned this in EWebLauncher/main.c,

+on_viewport_changed(void* user_data, Evas_Object* webview, void* event_info)
+{
+    w = (w == -1) ? DEFAULT_WIDTH : w;
+    h = (h == -1) ? DEFAULT_HEIGHT : h;
+    initScale = ((int)initScale == -1) ? DEFAULT_SCALE : initScale;
+    minScale = ((int)minScale == -1) ? DEFAULT_SCALE : minScale; 
+    maxScale = ((int)maxScale == -1) ? DEFAULT_SCALE : maxScale;
...


Gustavo,
I would like to know how do you think about new patch.