After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 669737 - ephy-web-view: cleanup close-web-view and close-requested signals
ephy-web-view: cleanup close-web-view and close-requested signals
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: General
unspecified
Other All
: Normal normal
: ---
Assigned To: Epiphany Maintainers
Epiphany Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-02-09 11:04 UTC by Diego Escalante Urrelo (not reading bugmail)
Modified: 2012-02-22 15:58 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
ephy-web-view: blank line nitpick (876 bytes, patch)
2012-02-09 11:04 UTC, Diego Escalante Urrelo (not reading bugmail) 		committed Details | Review
ephy-web-view: remove close-requested signal (6.50 KB, patch)
2012-02-09 11:04 UTC, Diego Escalante Urrelo (not reading bugmail) 		committed Details | Review
ephy-web-view: don't assume a embed-container in close-web-view (1.54 KB, patch)
2012-02-09 11:04 UTC, Diego Escalante Urrelo (not reading bugmail) 		committed Details | Review

Description Diego Escalante Urrelo (not reading bugmail) 2012-02-09 11:04:09 UTC 
Remove duplicated signal and cleanup the code of the handler of the
correct one.
Comment 1 Diego Escalante Urrelo (not reading bugmail) 2012-02-09 11:04:10 UTC 
Created attachment 207175 [details] [review]
ephy-web-view: blank line nitpick
Comment 2 Diego Escalante Urrelo (not reading bugmail) 2012-02-09 11:04:13 UTC 
Created attachment 207176 [details] [review]
ephy-web-view: remove close-requested signal

WebKitWebView has a ::close-web-view signal for the same thing.

The only user of this was ephy-window, for exactly the same thing that
we are already doing in ephy-web-view, when handling ::close-web-view.
Comment 3 Diego Escalante Urrelo (not reading bugmail) 2012-02-09 11:04:15 UTC 
Created attachment 207177 [details] [review]
ephy-web-view: don't assume a embed-container in close-web-view

The EphyWebView is not necessarily embedded in an EphyEmbedContainer.

Simplify close_web_view_cb and make it smarter. It now handles
EphyEmbedContainer and random toplevel widgets.
Comment 4 Xan Lopez 2012-02-10 10:35:27 UTC 
Review of attachment 207175 [details] [review]:

OK.
Comment 5 Xan Lopez 2012-02-10 10:37:11 UTC 
Review of attachment 207176 [details] [review]:

Are you saying this code never actually runs?
Comment 6 Diego Escalante Urrelo (not reading bugmail) 2012-02-10 20:12:37 UTC 
Comment on attachment 207175 [details] [review]
ephy-web-view: blank line nitpick

Attachment 207175 [details] pushed as 8be7e8d - ephy-web-view: blank line nitpick
Comment 7 Diego Escalante Urrelo (not reading bugmail) 2012-02-10 20:31:01 UTC 
(In reply to comment #5)
> Review of attachment 207176 [details] [review]:
> 
> Are you saying this code never actually runs?

Yes. No one uses the signal in the codebase, nor in epiphany-extensions.

There is also tab-close-request which does something similar, but is used, and close-web-view, which is handled in EphyWebView already.
Comment 8 Xan Lopez 2012-02-22 10:38:05 UTC 
(In reply to comment #7)
> Yes. No one uses the signal in the codebase, nor in epiphany-extensions.
> 
> There is also tab-close-request which does something similar, but is used, and
> close-web-view, which is handled in EphyWebView already.

Right, I see. I recently cleaned up a bit the code in EphyNotebook. It would be good to handle in just one place the whole "destroy the window when there's only one web view" thing.
Comment 9 Xan Lopez 2012-02-22 10:40:24 UTC 
Review of attachment 207176 [details] [review]:

OK.
Comment 10 Xan Lopez 2012-02-22 10:43:22 UTC 
Review of attachment 207177 [details] [review]:

OK, nice, so this will end up destroying the container when there's just one child left through tab-close-request in EphyWindow, correct? Looks like it, so go for it.
Comment 11 Diego Escalante Urrelo (not reading bugmail) 2012-02-22 15:58:39 UTC 
Attachment 207176 [details] pushed as f1fd22a - ephy-web-view: remove close-requested signal
Attachment 207177 [details] pushed as a24e4eb - ephy-web-view: don't assume a embed-container in close-web-view