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 780851 - [PATCH] Add bookmark followed by remove bookmark causes crash
[PATCH] Add bookmark followed by remove bookmark causes crash
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: Bookmarks
git master
Other Linux
: Normal normal
: ---
Assigned To: Epiphany Maintainers
Epiphany Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-04-02 18:40 UTC by Iulian Radu
Modified: 2017-04-02 19:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
add-bookmark-popover: Save bookmarks on both popover show and close (5.32 KB, patch)
2017-04-02 18:41 UTC, Iulian Radu
committed Details | Review

Description Iulian Radu 2017-04-02 18:40:17 UTC
Due to the fact that a bookmark is added to the manager when the popover is closed, a press on the bookmark button followed by a press on the "Remove bookmark" button causes a crash because ephy_bookmarks_manager_remove_bookmark() tries to find the new bookmark which has not been added yet.
Comment 1 Iulian Radu 2017-04-02 18:41:33 UTC
Created attachment 349146 [details] [review]
add-bookmark-popover: Save bookmarks on both popover show and close

Due to the fact that a bookmark is added to the manager when the popover
is closed, a press on the bookmark button followed by a press on the "Remove
bookmark" button causes a crash because ephy_bookmarks_manager_remove_bookmark()
tries to find the new bookmark which has not been added yet.

Revert to the original behaviour where a bookmark is saved immediately
after pressing the star, but also save the bookmark when the popover is
closed in case the user changed something while the popover was open
Comment 2 Michael Catanzaro 2017-04-02 19:05:40 UTC
Whatever you do to fix this is going to cause some new crash somewhere else. :( The solution to this is automated testing, but no chance of that happening anytime soon. Shame.
Comment 3 Michael Catanzaro 2017-04-02 19:06:23 UTC
Review of attachment 349146 [details] [review]:

OK
Comment 4 Iulian Radu 2017-04-02 19:10:49 UTC
Attachment 349146 [details] pushed as 22bdf16 - add-bookmark-popover: Save bookmarks on both popover show and close