Bug 151567 - REGRESSION(r192053): MiniBrowser doesn't exit when clicking on the close-window button
Summary: REGRESSION(r192053): MiniBrowser doesn't exit when clicking on the close-wind...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: WebKit Local Build
Hardware: All Linux
: P2 Normal
Assignee: Gyuyoung Kim
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-11-23 09:03 PST by Renata Hodovan
Modified: 2015-12-07 02:47 PST (History)
8 users (show)

See Also:


Attachments
Patch (1.34 KB, patch)
2015-11-23 22:19 PST, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch (1.41 KB, patch)
2015-11-23 22:34 PST, Gyuyoung Kim
no flags Details | Formatted Diff | Diff
Patch (1.34 KB, patch)
2015-11-23 22:52 PST, Gyuyoung Kim
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Renata Hodovan 2015-11-23 09:03:13 PST
Open www.google.com with MiniBrowser (debug or release) and try to close it with the close-window button in the corner. With an EFL build nothing happens for me (i.e. the window does not close). Could somebody test it with another port too?
Comment 1 BJ Burg 2015-11-23 15:15:14 PST
This does not seem to reproduce on Mac. Changing platform to EFL for now.
Comment 2 Gyuyoung Kim 2015-11-23 21:07:57 PST
This bug looks a regression.
Comment 3 Gyuyoung Kim 2015-11-23 22:19:34 PST
Created attachment 266120 [details]
Patch
Comment 4 Hunseop Jeong 2015-11-23 22:30:55 PST
Comment on attachment 266120 [details]
Patch

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

> Tools/MiniBrowser/efl/main.c:2040
> +    window_close(window_find_with_elm_window(elm_window));

If we don't use the parameters (user_data, event_info), how about removing them?
Comment 5 Gyuyoung Kim 2015-11-23 22:34:38 PST
Created attachment 266121 [details]
Patch
Comment 6 Gyuyoung Kim 2015-11-23 22:35:40 PST
Comment on attachment 266120 [details]
Patch

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

>> Tools/MiniBrowser/efl/main.c:2040
>> +    window_close(window_find_with_elm_window(elm_window));
> 
> If we don't use the parameters (user_data, event_info), how about removing them?

It is not huge related with this fix though, let's remove unused parameters as well.
Comment 7 Gyuyoung Kim 2015-11-23 22:52:16 PST
Created attachment 266124 [details]
Patch
Comment 8 Gyuyoung Kim 2015-11-23 23:08:17 PST
(In reply to comment #4)
> Comment on attachment 266120 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=266120&action=review
> 
> > Tools/MiniBrowser/efl/main.c:2040
> > +    window_close(window_find_with_elm_window(elm_window));
> 
> If we don't use the parameters (user_data, event_info), how about removing
> them?

EFL MiniBrowser is C, not C++. So we can't omit parameter.

http://stackoverflow.com/questions/8776810/parameter-name-omitted-c-vs-c
Comment 9 Hunseop Jeong 2015-11-23 23:13:43 PST
(In reply to comment #8)
> (In reply to comment #4)
> > Comment on attachment 266120 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=266120&action=review
> > 
> > > Tools/MiniBrowser/efl/main.c:2040
> > > +    window_close(window_find_with_elm_window(elm_window));
> > 
> > If we don't use the parameters (user_data, event_info), how about removing
> > them?
> 
> EFL MiniBrowser is C, not C++. So we can't omit parameter.
> 
> http://stackoverflow.com/questions/8776810/parameter-name-omitted-c-vs-c

Okay,, I just worry about some warning occur case like as c++.
Thanks for the information.
Comment 10 WebKit Commit Bot 2015-11-24 16:20:51 PST
Comment on attachment 266124 [details]
Patch

Clearing flags on attachment: 266124

Committed r192767: <http://trac.webkit.org/changeset/192767>
Comment 11 WebKit Commit Bot 2015-11-24 16:20:56 PST
All reviewed patches have been landed.  Closing bug.
Comment 12 Ryuan Choi 2015-12-07 02:47:49 PST
(In reply to comment #11)
> All reviewed patches have been landed.  Closing bug.

Sorry for the late answer.
I made a Bug 151934 to fix this regression.