It is very useful to start MiniBrowser in Legacy mode. e.g. to compare how a test looks in MiniBrowser to the pixel tests results.
Created attachment 176865 [details] Patch The new API name was suggested in https://bugs.webkit.org/show_bug.cgi?id=100674#c47
Comment on attachment 176865 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=176865&action=review > Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:213 > +void EwkContext::setViewBehavior(Ewk_View_Behavior behavior) > +{ > + m_behavior = behavior; > +} > + > +Ewk_View_Behavior EwkContext::viewBehavior() const > +{ > + return m_behavior; > +} Do we really want this to be public api? You could also just use a hidden environment variable. Every thing we expose has to be supported
Comment on attachment 176865 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=176865&action=review >> Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:213 >> +} > > Do we really want this to be public api? You could also just use a hidden environment variable. > > Every thing we expose has to be supported I agree with Kenneth. We should avoid to add unneeded APIs.
(In reply to comment #2) > (From update of attachment 176865 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=176865&action=review > > > Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:213 > > +void EwkContext::setViewBehavior(Ewk_View_Behavior behavior) > > +{ > > + m_behavior = behavior; > > +} > > + > > +Ewk_View_Behavior EwkContext::viewBehavior() const > > +{ > > + return m_behavior; > > +} > > Do we really want this to be public api? You could also just use a hidden environment variable. > > Every thing we expose has to be supported From https://bugs.webkit.org/show_bug.cgi?id=100674#c47 I assumed that we do, but this can be done without a new API.
Created attachment 176962 [details] Patch Removed the new API.
Comment on attachment 176962 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=176962&action=review > Source/WebKit2/ChangeLog:10 > + > + Allow calling WKViewCreate without creating a context first. A default context > + will be used. > + You should explain what that actually means. How is that related to the legacy mode. > Tools/MiniBrowser/efl/main.c:1063 > - window->ewk_view = ewk_view_smart_add(evas, smart, ewk_context_default_get()); > + window->ewk_view = legacy_behavior_enabled ? (Evas_Object*)WKViewCreate(evas, 0, 0) : ewk_view_smart_add(evas, smart, ewk_context_default_get()); Why not make this more clean 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 window->ewk_view = ewk_view_smart_add(evas, smart, ewk_context_default_get());
(In reply to comment #6) > (From update of attachment 176962 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=176962&action=review > > > Source/WebKit2/ChangeLog:10 > > + > > + Allow calling WKViewCreate without creating a context first. A default context > > + will be used. > > + > > You should explain what that actually means. How is that related to the legacy mode. > > > Tools/MiniBrowser/efl/main.c:1063 > > - window->ewk_view = ewk_view_smart_add(evas, smart, ewk_context_default_get()); > > + window->ewk_view = legacy_behavior_enabled ? (Evas_Object*)WKViewCreate(evas, 0, 0) : ewk_view_smart_add(evas, smart, ewk_context_default_get()); > > Why not make this more clean > > 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 > window->ewk_view = ewk_view_smart_add(evas, smart, ewk_context_default_get()); Because people usually ask me to condense lines as much as possible :)
> > 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 > > window->ewk_view = ewk_view_smart_add(evas, smart, ewk_context_default_get()); > Because people usually ask me to condense lines as much as possible :) We are doing something a bit out of the ordinary here, let's make that clear.
Comment on attachment 176962 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=176962&action=review >> Tools/MiniBrowser/efl/main.c:1063 >> + window->ewk_view = legacy_behavior_enabled ? (Evas_Object*)WKViewCreate(evas, 0, 0) : ewk_view_smart_add(evas, smart, ewk_context_default_get()); > > Why not make this more clean > > 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 > window->ewk_view = ewk_view_smart_add(evas, smart, ewk_context_default_get()); As you don't add the smart now, why create it?
Created attachment 176971 [details] Patch Address Kenneth's comments
Comment on attachment 176971 [details] Patch Clearing flags on attachment: 176971 Committed r136320: <http://trac.webkit.org/changeset/136320>
All reviewed patches have been landed. Closing bug.