[Webkit-unassigned] [Bug 61850] [EFL] Add MiniBrowserEfl.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=61850





--- Comment #8 from Ryuan Choi <ryuan.choi at samsung.com>  2011-06-01 06:46:50 PST ---
(In reply to comment #2)
> I think it is better to insert CMakeListEfl.txt into efl directory. MiniBrowser of gtk, qt have build script in its directory.

It's my mistake. I moved it into efl directory.

(In reply to comment #3)
> (From update of attachment 95572 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=95572&action=review
> 
> Once WebKit2 is compiled in EFL port, I think this can enter. I agree with Gyuyoung that the cmake file should be in efl directory. But we can do a bit better and split it in a generic CMakeLists.txt file and another efl/CMakeListsEfl.txt. This way other ports that use cmake can profit from this work too.
> 
Done.
It's my mistake.

> > Tools/MiniBrowser/CMakeListsEfl.txt:15
> > +    ${WEBKIT2_DIR}/UIProcess/API/efl
> > +    ${WEBKIT2_DIR}
> > +    ${DERIVED_SOURCES_WEBKIT2_DIR}/include
> 
> Since we don't support webkit2, are you sure you need this?
> 
> > Tools/MiniBrowser/efl/main.c:55
> > +static int browserCreate(const char *url, const char *engine)
> > +{
> > +    MiniBrowser *app = (MiniBrowser*) malloc(sizeof(MiniBrowser));
> > +
> > +    app->ee = ecore_evas_software_x11_new(NULL, 0, 0, 0, DEFAULT_WIDTH, DEFAULT_HEIGHT);
> 
> Could you use ecore_evas_new() here instead?
Done.

> 
> > Tools/MiniBrowser/efl/main.c:56
> > +    ecore_evas_title_set(app->ee, "Samsung MiniBrowser");
> 
> I think it's not good to let Samsung here. WebKit would be a better name.
> 
> > Tools/MiniBrowser/efl/main.c:71
> > +    /* Create webview */
> > +    app->browser = ewk_view_add(app->evas, WKContextGetSharedProcessContext(), 0);
> 
> It seems you are indeed using webkit2. I'm wondering what's happening since we don't have support to webkit2 yet.
> 
> > Tools/MiniBrowser/efl/main.c:97
> > +    browserCreate(url, engine);
> > +
> > +    ecore_event_handler_add(ECORE_EVENT_SIGNAL_EXIT, mainSignalExit, NULL);
> 
> Since in this file we are following EFL style, I think it's better not to use camelCase for function names.

I changed it EFL style. but we already use browserCreate. Is it ok?

(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 95572 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=95572&action=review
> > 
> > Once WebKit2 is compiled in EFL port, I think this can enter. I agree with Gyuyoung that the cmake file should be in efl directory. But we can do a bit better and split it in a generic CMakeLists.txt file and another efl/CMakeListsEfl.txt. This way other ports that use cmake can profit from this work too.
> > 
> > > Tools/MiniBrowser/CMakeListsEfl.txt:15
> > > +    ${WEBKIT2_DIR}/UIProcess/API/efl
> > > +    ${WEBKIT2_DIR}
> > > +    ${DERIVED_SOURCES_WEBKIT2_DIR}/include
> > 
> > Since we don't support webkit2, are you sure you need this?
> 
> I've just seen you already answered this on your first comment. Sorry for the noise.

No problem.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.