Bug 103860 – [WK2][EFL] Add ewk API for creating a view with legacy behavior

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebKit EFL

[KyungTae Kim]

The EFL WebKit2 MiniBrowser uses WK2 API for creating a view with legacy behavior (bug 103679).

But, it seems not a good way because it's the only WK2 API that is used by the MiniBrowser, 
and it needs a type conversion between incompatible types that makes the below build warning:
 
Tools/MiniBrowser/efl/main.c:1064:28: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]

How about add a ewk API for creating a view with legacy behavior?

[KyungTae Kim]

Created attachment 177192 [details]
Patch

[Yael]

Please see https://bugs.webkit.org/show_bug.cgi?id=103679#c2 and https://bugs.webkit.org/show_bug.cgi?id=103679#c3.

[KyungTae Kim]

(In reply to comment #2)
> Please see https://bugs.webkit.org/show_bug.cgi?id=103679#c2 and https://bugs.webkit.org/show_bug.cgi?id=103679#c3.

I agree with the comments. The 'setViewBehavior' and 'viewBehavior' should not to be APIs.

What I suggest is a API for just what you want - creating a view with legacy behavior.
(not 'setViewBehavior' for already created view)

You need that functionality but there was no ewk_ API for that, so that API is necessary API I think.

I think the alternative implementation would be the view creating API with behavior like below:

enum Ewk_View_Behavior {
    EWK_VIEW_BEHAVIOR_DEFAULT,
    EWK_VEIW_BEHAVIOR_LEGACY
};
 ewk_view_smart_add_with_behavior(evas, smart, ewk_context_default_get(), EWK_VEIW_BEHAVIOR_LEGACY);

[Gyuyoung Kim]

(In reply to comment #3)
> (In reply to comment #2)
> > Please see https://bugs.webkit.org/show_bug.cgi?id=103679#c2 and https://bugs.webkit.org/show_bug.cgi?id=103679#c3.
> 
> I agree with the comments. The 'setViewBehavior' and 'viewBehavior' should not to be APIs.
> 
> What I suggest is a API for just what you want - creating a view with legacy behavior.
> (not 'setViewBehavior' for already created view)
> 
> You need that functionality but there was no ewk_ API for that, so that API is necessary API I think.
> 
> I think the alternative implementation would be the view creating API with behavior like below:
> 
> enum Ewk_View_Behavior {
>     EWK_VIEW_BEHAVIOR_DEFAULT,
>     EWK_VEIW_BEHAVIOR_LEGACY
> };
>  ewk_view_smart_add_with_behavior(evas, smart, ewk_context_default_get(), EWK_VEIW_BEHAVIOR_LEGACY);

I don't agree to change API by MiniBrowser needs. Do you think this will be needed by real application(e.g. browser) ?  Kenneth, what do you think ?

[KyungTae Kim]

(In reply to comment #4)
> I don't agree to change API by MiniBrowser needs. Do you think this will be needed by real application(e.g. browser) ?  Kenneth, what do you think ?

I think this will be needed when we make a PC Browser with legacy behavior.

[Mikhail Pozdnyakov]

Think "legacy" means that it's not supposed to be used any more :) Don't think we should expose it.

[Ryuan Choi]

(In reply to comment #6)
> Think "legacy" means that it's not supposed to be used any more :) Don't think we should expose it.

By the way, exposing WK is also bad.

We didn't expose it to application side.

[KyungTae Kim]

(In reply to comment #6)
> Think "legacy" means that it's not supposed to be used any more :) Don't think we should expose it.

If the  "legacy" is "not supposed to be used", the WKViewCreate also need to do "default behavior" by default, not "legacy behavior".
Anyway, the "legacy behavior" is needed for Minibrowser now, and may be needed for other applications that don't want to use the backing store.

[Kenneth Rohde Christiansen]

(In reply to comment #8)
> (In reply to comment #6)
> > Think "legacy" means that it's not supposed to be used any more :) Don't think we should expose it.
> 
> If the  "legacy" is "not supposed to be used", the WKViewCreate also need to do "default behavior" by default, not "legacy behavior".
> Anyway, the "legacy behavior" is needed for Minibrowser now, and may be needed for other applications that don't want to use the backing store.

If you don't want a proper browser featuring high performance features commonly known as HTML5 today, you can always use a WebKit snapshot from 2008 or earlier.

Joke aside, I mean we cannot and don't want to support all kind of configurations and especially not ones that means sacrificing the platform. Even browsers like Chrome today uses a backing store by default. Get over it. There should not be any "may be used". We don't care about "may", we need to focus on what we "do need".

[Kenneth Rohde Christiansen]

Comment on attachment 177192 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=177192&action=review

> Source/WebKit2/UIProcess/API/efl/ewk_view.h:342
>  EAPI Evas_Object *ewk_view_smart_add(Evas *e, Evas_Smart *smart, Ewk_Context *context);
> +EAPI Evas_Object *ewk_view_smart_add_with_legacy_behavior(Evas *e, Evas_Smart *smart, Ewk_Context *context);

We decided to not expose these things publically.

> Tools/MiniBrowser/efl/main.c:1062
> -    if (legacy_behavior_enabled) {
> -        // Use raw WK2 api to create a view using legacy mode.
> -        window->ewk_view = (Evas_Object*)WKViewCreate(evas, 0, 0);
> -    } else {
> -        Evas_Smart *smart = evas_smart_class_new(&ewkViewClass->sc);
> +

MiniBrowser is a development tool. There is no problem using some internal API for the sake of being able to improve debugging some cases.

The API is internal, it will not be documented, and even if people find it and use it, we have no guarantees it will stay there or not change behavior. That is fine.

[KyungTae Kim]

(In reply to comment #10)
> MiniBrowser is a development tool. There is no problem using some internal API for the sake of being able to improve debugging some cases.
> The API is internal, it will not be documented, and even if people find it and use it, we have no guarantees it will stay there or not change behavior. That is fine.

OK. I understood your point. I'll close this bug.
Then, I think the below warning need to be fixed in another way : 
Tools/MiniBrowser/efl/main.c:1064:28: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]