[Webkit-unassigned] [Bug 40278] [EFL] EFLWebKit doesn't support viewport meta tag
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Jun 14 13:12:38 PDT 2010
https://bugs.webkit.org/show_bug.cgi?id=40278
--- Comment #20 from Lucas De Marchi <lucas.demarchi at profusion.mobi> 2010-06-14 13:12:35 PST ---
(From update of attachment 58645)
>Index: WebKit/ChangeLog
>===================================================================
>--- WebKit/ChangeLog (revision 61120)
>+++ WebKit/ChangeLog (working copy)
>@@ -1,3 +1,35 @@
>+2010-06-14 Gyuyoung Kim <gyuyoung.kim at samsung.com>
>+
>+ Reviewed by NOBODY (OOPS!).
>+
>+ [EFL] EFLWebKit doesn't support viewport meta tag.
Please describe the patch here, not the but title.
>+ https://bugs.webkit.org/show_bug.cgi?id=40278
>+
>+ Support viewport meta tag on EFL Port.
>+
>+ * efl/EWebLauncher/main.c:
>+ (on_viewport_changed):
>+ (browserCreate):
>+ * efl/WebCoreSupport/ChromeClientEfl.cpp:
>+ (WebCore::ChromeClientEfl::didReceiveViewportArguments):
>+ * efl/WebCoreSupport/ChromeClientEfl.h:
>+ * efl/WebCoreSupport/FrameLoaderClientEfl.cpp:
>+ (WebCore::FrameLoaderClientEfl::dispatchDidCommitLoad):
>+ (WebCore::FrameLoaderClientEfl::dispatchDidFirstVisuallyNonEmptyLayout):
>+ * efl/ewk/ewk_private.h:
>+ * efl/ewk/ewk_view.cpp:
>+ (_ewk_view_priv_new):
>+ (ewk_view_zoom_set):
>+ (ewk_view_zoom_weak_set):
>+ (ewk_view_zoom_animated_set):
>+ (ewk_view_init_layout_completed_set):
>+ (ewk_view_init_layout_completed_get):
>+ (ewk_view_viewport_set):
>+ (ewk_view_viewport_get):
>+ (ewk_view_zoom_range_set):
>+ (ewk_view_user_scalable_set):
>+ * efl/ewk/ewk_view.h:
>+
Comments about the changed functions are still missing.
> 2010-06-14 Ilya Tikhonovsky <loislo at chromium.org>
>
> Reviewed by Pavel Feldman.
>Index: WebKit/efl/EWebLauncher/main.c
>===================================================================
>--- WebKit/efl/EWebLauncher/main.c (revision 61116)
>+++ WebKit/efl/EWebLauncher/main.c (working copy)
>@@ -47,6 +47,7 @@
>
> #define DEFAULT_WIDTH 800
> #define DEFAULT_HEIGHT 600
>+#define DEFAULT_SCALE 1.0
>
> #define info(format, args...) \
> do { \
>@@ -346,6 +347,27 @@ on_tooltip_text_set(void* user_data, Eva
> }
>
> static void
>+on_viewport_changed(void* user_data, Evas_Object* webview, void* event_info)
>+{
>+ int w, h;
>+ float initScale, minScale, maxScale;
>+ Eina_Bool userScalable;
>+
>+ ewk_view_viewport_get(webview, &w, &h, &initScale, &maxScale, &minScale, &userScalable);
>+
>+ 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;
imo these two are wrong. The default should be ZOOM_MIN/ZOOM_MAX respectively. Otherwise, if the page does not have a viewport meta tag it will not possible to set the zoom because of checks made in ewk_view_zoom_set() and functions alike.
>+
>+ ewk_view_fixed_layout_size_set(webview, w, h);
>+ ewk_view_zoom_set(webview, initScale, 0, 0);
>+ ewk_view_zoom_range_set(webview, minScale, maxScale);
>+ ewk_view_user_scalable_set(webview, userScalable);
>+}
What if I don't want to be forced to apply the new viewport arguments? There's no way as of now to ignore the meta tag.
>+void ChromeClientEfl::didReceiveViewportArguments(Frame* frame, const ViewportArguments& arguments) const
>+{
>+ Eina_Bool userScalable;
>+
>+ userScalable = (int)arguments.userScalable == 1 ? EINA_TRUE : EINA_FALSE;
You should check for arguments.userScalable == -1, not just == 1.
>+ ewk_view_viewport_set(m_view, (int)arguments.width, (int)arguments.height, arguments.initialScale, arguments.minimumScale, arguments.maximumScale, userScalable);
>+ ewk_view_init_layout_completed_set(m_view, EINA_TRUE);
Since the only place where you need this is inside ChromeClientEfl/FrameLoaderClientEfl, it's not necessary (neither desired) to have this function as an ewk_view_*. Just add a boolean field to FrameLoaderClientEfl to check/set if the signal has already been sent.
>Index: WebKit/efl/WebCoreSupport/FrameLoaderClientEfl.cpp
>===================================================================
>@@ -580,6 +581,8 @@ void FrameLoaderClientEfl::dispatchDidCo
> return;
> ewk_view_title_set(m_view, 0);
> ewk_view_uri_changed(m_view);
>+
>+ ewk_view_init_layout_completed_set(m_view, EINA_FALSE);
Same here, use a field in this class.
>Index: WebKit/efl/ewk/ewk_private.h
>===================================================================
>--- WebKit/efl/ewk/ewk_private.h (revision 61116)
>+++ WebKit/efl/ewk/ewk_private.h (working copy)
>@@ -93,6 +93,10 @@ WTF::PassRefPtr<WebCore::Widget> ewk_vie
>
> void ewk_view_popup_new(Evas_Object *o, WebCore::PopupMenuClient* client, int selected, const WebCore::IntRect& rect);
>
>+void ewk_view_viewport_set(Evas_Object *o, int w, int h, float init_scale, float max_scale, float min_scale, Eina_Bool user_scalable);
>+void ewk_view_init_layout_completed_set(Evas_Object *o, Eina_Bool init_layout);
>+Eina_Bool ewk_view_init_layout_completed_get(Evas_Object *o);
As mentioned above, I think it's better if we don't have these two .
>Index: WebKit/efl/ewk/ewk_view.cpp
>===================================================================
>--- WebKit/efl/ewk/ewk_view.cpp (revision 61116)
>+++ WebKit/efl/ewk/ewk_view.cpp (working copy)
>@@ -100,6 +100,20 @@ struct _Ewk_View_Private_Data {
> Eina_Bool resizable_textareas:1;
> Eina_Bool private_browsing:1;
> Eina_Bool caret_browsing:1;
>+ Eina_Bool init_layout_completed:1;
>+ Eina_Bool user_scalable:1;
>+ struct {
>+ int w;
>+ int h;
>+ float init_scale;
>+ float min_scale;
>+ float max_scale;
>+ Eina_Bool user_scalable:1;
You have 2 user_scalable... one inside and one outside viewport.
>+ } viewport;
>+ struct {
>+ float min_scale;
>+ float max_scale;
>+ } zoom_range;
> } settings;
> struct {
> struct {
>@@ -574,6 +588,9 @@ static Ewk_View_Private_Data* _ewk_view_
> priv->settings.private_browsing = priv->page_settings->privateBrowsingEnabled();
> priv->settings.caret_browsing = priv->page_settings->caretBrowsingEnabled();
>
>+ priv->settings.zoom_range.min_scale = ZOOM_MIN;
>+ priv->settings.zoom_range.max_scale = ZOOM_MAX;
>+
Why did you set these parameters and didn't set the others? This would be set anyway when the first page is loaded, even when it does not have a viewport tag, doesn't it?
> priv->main_frame = _ewk_view_core_frame_new(sd, priv, 0).get();
> if (!priv->main_frame) {
> CRITICAL("Could not create main frame.");
>@@ -1754,14 +1771,22 @@ float ewk_view_zoom_get(const Evas_Objec
> Eina_Bool ewk_view_zoom_set(Evas_Object* o, float zoom, Evas_Coord cx, Evas_Coord cy)
> {
> EWK_VIEW_SD_GET_OR_RETURN(o, sd, EINA_FALSE);
>+ EWK_VIEW_PRIV_GET(sd, priv);
>+
> EINA_SAFETY_ON_NULL_RETURN_VAL(sd->api, EINA_FALSE);
> EINA_SAFETY_ON_NULL_RETURN_VAL(sd->api->zoom_set, EINA_FALSE);
>- if (zoom < ZOOM_MIN) {
>- WRN("zoom level is < %f : %f", (double)ZOOM_MIN, (double)zoom);
>+
>+ if (!priv->settings.user_scalable) {
Again, there must be a way to ignore the tag.
>+ WRN("userScalable is false");
> return EINA_FALSE;
> }
>- if (zoom > ZOOM_MAX) {
>- WRN("zoom level is > %f : %f", (double)ZOOM_MAX, (double)zoom);
>+
>+ if (zoom < priv->settings.zoom_range.min_scale) {
priv->settings.zoom_range.min_scale can be == -1 if page didn't add the tag. Therefore, You must check if priv->settings.zoom_range.min_scale != -1 before comparing to zoom. something like this:
if (priv->settings.zoom_range.min_scale >= 0.0 && zoom < priv->settings.zoom_range.min_scale) {
>+ WRN("zoom level is < %f : %f", (double)priv->settings.zoom_range.min_scale, (double)zoom);
>+ return EINA_FALSE;
>+ }
>+ if (zoom > priv->settings.zoom_range.max_scale) {
Here you have to put the same check as above.
>+ WRN("zoom level is > %f : %f", (double)priv->settings.zoom_range.max_scale, (double)zoom);
> return EINA_FALSE;
> }
>
>@@ -1824,14 +1849,22 @@ void ewk_view_zoom_weak_smooth_scale_set
> Eina_Bool ewk_view_zoom_weak_set(Evas_Object* o, float zoom, Evas_Coord cx, Evas_Coord cy)
> {
> EWK_VIEW_SD_GET_OR_RETURN(o, sd, EINA_FALSE);
>+ EWK_VIEW_PRIV_GET(sd, priv);
>+
> EINA_SAFETY_ON_NULL_RETURN_VAL(sd->api, EINA_FALSE);
> EINA_SAFETY_ON_NULL_RETURN_VAL(sd->api->zoom_weak_set, EINA_FALSE);
>- if (zoom < ZOOM_MIN) {
>- WRN("zoom level is < %f : %f", (double)ZOOM_MIN, (double)zoom);
>+
>+ if (!priv->settings.user_scalable) {
>+ WRN("userScalable is false");
>+ return EINA_FALSE;
>+ }
>+
>+ if (zoom < priv->settings.zoom_range.min_scale) {
check here as above.
>+ WRN("zoom level is < %f : %f", (double)priv->settings.zoom_range.min_scale, (double)zoom);
> return EINA_FALSE;
> }
>- if (zoom > ZOOM_MAX) {
>- WRN("zoom level is > %f : %f", (double)ZOOM_MAX, (double)zoom);
>+ if (zoom > priv->settings.zoom_range.max_scale) {
check here as above.
>@@ -1974,12 +2007,17 @@ Eina_Bool ewk_view_zoom_animated_set(Eva
> EINA_SAFETY_ON_NULL_RETURN_VAL(sd->api, EINA_FALSE);
> EINA_SAFETY_ON_NULL_RETURN_VAL(sd->api->zoom_weak_set, EINA_FALSE);
>
>- if (zoom < ZOOM_MIN) {
>- WRN("zoom level is < %f : %f", (double)ZOOM_MIN, (double)zoom);
>+ if (!priv->settings.user_scalable) {
>+ WRN("userScalable is false");
> return EINA_FALSE;
> }
>- if (zoom > ZOOM_MAX) {
>- WRN("zoom level is > %f : %f", (double)ZOOM_MAX, (double)zoom);
>+
>+ if (zoom < priv->settings.zoom_range.min_scale) {
check here as above.
>+ WRN("zoom level is < %f : %f", (double)priv->settings.zoom_range.min_scale, (double)zoom);
>+ return EINA_FALSE;
>+ }
>+ if (zoom > priv->settings.zoom_range.max_scale) {
check here as above.
<...>
>Index: WebKit/efl/ewk/ewk_view.h
>===================================================================
>--- WebKit/efl/ewk/ewk_view.h (revision 61116)
>+++ WebKit/efl/ewk/ewk_view.h (working copy)
>@@ -83,6 +83,7 @@ extern "C" {
> * - "download,request", Ewk_Download: reports a download is being requested
> * and as arguments gives its details.
> * - "icon,received", void: main frame received an icon.
>+ * - "viewport,changed", void: Report that viewport was changed.
> */
>
> typedef struct _Ewk_View_Smart_Data Ewk_View_Smart_Data;
>@@ -451,6 +452,10 @@ EAPI void ewk_view_paint_context_transla
> EAPI Eina_Bool ewk_view_paint(Ewk_View_Private_Data *priv, cairo_t *cr, const Eina_Rectangle *area);
> EAPI Eina_Bool ewk_view_paint_contents(Ewk_View_Private_Data *priv, cairo_t *cr, const Eina_Rectangle *area);
>
>+EAPI void ewk_view_viewport_get(Evas_Object *o, int* w, int* h, float* init_scale, float* max_scale, float* min_scale, Eina_Bool* user_scalable);
>+EAPI void ewk_view_zoom_range_set(Evas_Object* o, float min_scale, float max_scale);
>+EAPI void ewk_view_user_scalable_set(Evas_Object* o, Eina_Bool user_scalable);
Missing the getters for these functions.
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list