GNOME Bugzilla – Bug 755711
Shift + control crash
Last modified: 2016-09-02 19:09:30 UTC
Steps to reproduce: 1. Select a file or folder in Nautilus. 2. Hold shift and control. 3. With those buttons still held down, press the down arrow key. Result: Nautilus terminates with a SIGABRT.
*** Bug 765527 has been marked as a duplicate of this bug. ***
It works fine for me with files. But with folders it crashes. I think it’s related to https://git.gnome.org/browse/nautilus/tree/src/nautilus-mime-actions.c#n1666
Thanks for taking the time to report this. Without a stack trace from the crash it's very hard to determine what caused it. Can you get us a stack trace? Please see https://wiki.gnome.org/Community/GettingInTouch/Bugzilla/GettingTraces for more information on how to do so. When pasting a stack trace in this bug report, please reset the status of this bug report from NEEDINFO to its previous status. Thanks in advance!
Created attachment 327344 [details] [review] Fix shift+control+down seg fault on a folder Using this shortcut on one or more folders caused segmentation fault. In order to solve this, it was added an extra flag to the function that activates the files, NAUTILUS_WINDOW_OPEN_FLAG_NEW_WINDOW. Also, to make sure that only the window from which the shortcut was launched is closed, the flag NAUTILUS_WINDOW_OPEN_FLAG_CLOSE_BEHIND is set only for the first call of nautilus_application_open_location_full.
Created attachment 327520 [details] [review] mime-actions: Fix shift+control+down seg fault on a folder Using this shortcut on one or more folders caused segmentation fault. In order to solve this, if there is a directory that needs to be activated, the NEW_WINDOW flag is set, so that the initial window will be closed and a new one will be created. After a new window is created, the CLOSE_BEHIND flag is disabled, so that the window will not be closed. The flags NEW_TAB and DONT_MAKE_ACTIVE are also set there, in order for new tabs to open in the newly created window.
Created attachment 327568 [details] [review] mime-actions: Fix shift+control+down seg fault on a folder Using this shortcut on one or more folders caused segmentation fault. In order to solve this, if there is a directory that needs to be activated, the NEW_WINDOW flag is set, so that the initial window will be closed and a new one will be created. After a new window is created, the CLOSE_BEHIND flag is disabled, so that the window will not be closed. The flags NEW_TAB and DONT_MAKE_ACTIVE are also set there(unless we want all directories opened in new windows), in order for new tabs to open in the newly created window.
Review of attachment 327568 [details] [review]: Starts looking good! Here some review: ::: src/nautilus-mime-actions.c @@ +1520,3 @@ gint num_unhandled; gint num_files; + gboolean open_files, closed_window; One var per line @@ +1639,3 @@ + { + flags |= NAUTILUS_WINDOW_OPEN_FLAG_NEW_TAB; + flags |= NAUTILUS_WINDOW_OPEN_FLAG_DONT_MAKE_ACTIVE; I would let this flag for the callers. We are not sure what the caller wants, for example, when opening from the sidebar. Any reason you added this here instead than on the caller? Also, I can see that this flag start looking like a bad decision on my part, and we should probably do the opossite, "make_active". But let's leave this for another patch. @@ +1650,3 @@ + /* if we want to close the window and activate a single directory, then we will need + * the NEW_WINDOW flag set */ + if (count == 1 && (parameters->flags & NAUTILUS_WINDOW_OPEN_FLAG_CLOSE_BEHIND) != 0) the count == 1 is pointless here, isn't it? @@ +1689,3 @@ + /* close only the window from which the action was launched and then open + * tabs/windows (depending on parameters->flags) */ + if (count >= 1 && !closed_window && count >= 1 is pointless... we don't call this with no files to activate... and in case we want to guard against this, we should do it at the start of the function. @@ +1700,3 @@ + + flags |= NAUTILUS_WINDOW_OPEN_FLAG_NEW_TAB; + flags |= NAUTILUS_WINDOW_OPEN_FLAG_DONT_MAKE_ACTIVE; The same, we don't know about this. What is the result you want to achieve? @@ +1711,3 @@ + + if (closed_window) + flags |= NAUTILUS_WINDOW_OPEN_FLAG_CLOSE_BEHIND; This is done previously with the flags &= (~NAUTILUS_WINDOW_OPEN_FLAG_CLOSE_BEHIND); no?
setting the flags NAUTILUS_WINDOW_OPEN_FLAG_NEW_TAB and NAUTILUS_WINDOW_OPEN_FLAG_DONT_MAKE_ACTIVE together creates the problem that if a user wants to open a new tab and make it active, that won't be possible.
Created attachment 327770 [details] [review] mime-actions: Fix shift+control+down seg fault on a folder Using this shortcut on one or more folders caused segmentation fault. In order to solve this, if there is a directory that needs to be activated, the NEW_WINDOW flag is set, so that the initial window will be closed and a new one will be created. After a new window is created, the CLOSE_BEHIND flag is disabled, so that the window will not be closed. The flags NEW_TAB and DONT_MAKE_ACTIVE are also set there(unless we want all directories opened in new windows), in order for new tabs to open in the newly created window.
Created attachment 327772 [details] [review] mime-actions: Fix flags in activate_files The problem is that if activate_files is called in order to activate a folder in a new tab and that folder should be active, that is not possible due to the fact that NEW_TAB and DONT_MAKE_ACTIVE were set together in the function activate_files.` We shouldn't set the DONT_MAKE_ACTIVE flag in activate_files and leave that flag to be set by the caller.
Review of attachment 327770 [details] [review]: Looks good except some style nitpicks, thanks! ::: src/nautilus-mime-actions.c @@ +1635,3 @@ + /* if CLOSE_BEHIND is set and have a directory to be activated, we will first have + * to open a new window and after that we can open the rest of files in tabs */ + if ((parameters->flags & NAUTILUS_WINDOW_OPEN_FLAG_CLOSE_BEHIND) != 0) if () { when the else more than two lines @@ +1637,3 @@ + if ((parameters->flags & NAUTILUS_WINDOW_OPEN_FLAG_CLOSE_BEHIND) != 0) + flags |= NAUTILUS_WINDOW_OPEN_FLAG_NEW_WINDOW; + else else { @@ +1647,3 @@ } } + else else { @@ +1690,3 @@ + /* close only the window from which the action was launched and then open + * tabs/windows (depending on parameters->flags) */ + if (!closed_window && (flags & NAUTILUS_WINDOW_OPEN_FLAG_CLOSE_BEHIND) != 0) if { @@ +1696,3 @@ + /* if NEW_WINDOW is set, we want all files in new windows, not in tabs */ + if ((parameters->flags & NAUTILUS_WINDOW_OPEN_FLAG_NEW_WINDOW) == 0) + { if {
Review of attachment 327772 [details] [review]: LGTM
Created attachment 334630 [details] [review] mime-actions: Fix shift+control+down seg fault on a folder Using this shortcut on one or more folders causes segmentation fault. In order to solve this, if there is a directory that needs to be activated, the NEW_WINDOW flag is set, so that the initial window will be closed and a new one will be created. After a new window is created, the CLOSE_BEHIND flag is disabled,so that the window will not be closed. The flags NEW_TAB is also set there(unless we want all directories opened in new windows), in order for new tabs to open in the newly created window.
Review of attachment 334630 [details] [review]: oh we forgot to commit this one, yeah looks good, thanks for updating to the latest code style!
Attachment 334630 [details] pushed as 5815ba1 - mime-actions: Fix shift+control+down seg fault on a folder