Bug 47092 - Removed unused callback in webkitwebview
Summary: Removed unused callback in webkitwebview
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-10-04 08:55 PDT by Carlos Garcia Campos
Modified: 2010-10-09 03:28 PDT (History)
2 users (show)

See Also:


Attachments
Remove unused callback (1.12 KB, patch)
2010-10-04 08:55 PDT, Carlos Garcia Campos
xan.lopez: review-
Details | Formatted Diff | Diff
Updated patch (2.00 KB, patch)
2010-10-07 03:05 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Another update (1.91 KB, patch)
2010-10-08 06:20 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Another iteration (1.91 KB, patch)
2010-10-08 06:23 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2010-10-04 08:55:51 PDT
Created attachment 69639 [details]
Remove unused callback

In webkit_web_view_forward_context_menu_event() a callback is connected for the destroy signal of the popup menu, but the menu is never destroyed so the callback never gets called. If it were called it would crash since NULL is passed as user_data, so I think it's safe to just remove the callback.
Comment 1 Xan Lopez 2010-10-05 03:08:56 PDT
Comment on attachment 69639 [details]
Remove unused callback

Looks good, but we need the ChangeLog.
Comment 2 Carlos Garcia Campos 2010-10-07 03:05:43 PDT
Created attachment 70063 [details]
Updated patch

It just adds a changelog entry
Comment 3 Xan Lopez 2010-10-07 04:59:03 PDT
Comment on attachment 70063 [details]
Updated patch

I think it's worth explaining in the comment why it's never called. We reuse the same menu always, holding a ref to it, so it's not destroyed when withdrawn by the user.
Comment 4 Carlos Garcia Campos 2010-10-08 06:20:09 PDT
Created attachment 70237 [details]
Another update

Includes the explation suggested by Xan to ChangeLog
Comment 5 Carlos Garcia Campos 2010-10-08 06:23:03 PDT
Created attachment 70239 [details]
Another iteration

I forgot the [GTK] prefix in the changelog, sorry.
Comment 6 Xan Lopez 2010-10-09 02:39:50 PDT
Comment on attachment 70239 [details]
Another iteration

Great, thanks.
Comment 7 WebKit Commit Bot 2010-10-09 03:28:31 PDT
Comment on attachment 70239 [details]
Another iteration

Clearing flags on attachment: 70239

Committed r69444: <http://trac.webkit.org/changeset/69444>
Comment 8 WebKit Commit Bot 2010-10-09 03:28:36 PDT
All reviewed patches have been landed.  Closing bug.