GNOME Bugzilla – Bug 781552
Exit selection mode only after printing has begun
Last modified: 2017-05-11 21:44:02 UTC
Ever since the selection-mode state was converted into a GAction (commit e8b2d3431e44d30e4f8edb2c67c600784b81d567), merely clicking the 'print' button on the selection toolbar exists the selection mode. eg., we don't want to leave the selection mode if the print dialog was cancelled. We should only leave it if an affirmative action has been taken. This matches the behaviour of the other modal dialogs - OrganizeCollectionDialog, PropertiesDialog and SharingDialog.
Created attachment 350156 [details] [review] documents, selections: Exit selection only after printing has begun
(In reply to Debarshi Ray from comment #0) > commit > e8b2d3431e44d30e4f8edb2c67c600784b81d567), Looks like the regular expression doesn't work across line breaks. :) commit e8b2d3431e44d30e4f8edb2c67c600784b81d567 Author: Cosimo Cecchi <cosimoc@gnome.org> Date: Sun Oct 30 21:33:15 2016 -0700 Move selection-mode state to a GAction The action is owned by the view. This allows us to clean up things quite a bit, and use the action accelerators instead of a custom event listener.
Review of attachment 350156 [details] [review]: ::: src/selections.js @@ +1053,3 @@ + function(doc) { + this._overview.getAction('selection-mode').change_state(GLib.Variant.new('b', false)); + doc.disconnect(beginPrintId); Should the signal also not be disconnected when exiting selection mode? Otherwise since the document may outlive the toolbar, we may end up connecting to this signal twice.
(In reply to Cosimo Cecchi from comment #3) > Review of attachment 350156 [details] [review] [review]: > > ::: src/selections.js > @@ +1053,3 @@ > + function(doc) { > + > this._overview.getAction('selection-mode').change_state(GLib.Variant.new('b', > false)); > + doc.disconnect(beginPrintId); > > Should the signal also not be disconnected when exiting selection mode? > Otherwise since the document may outlive the toolbar, we may end up > connecting to this signal twice. Good point. I think that we should disconnect when SelectionToolbar is hidden or destroyed. Otherwise, it will get tricky if, for some reason, SelectionToolbar is hidden or destroyed before the state of the selection-mode GAction changes.
Created attachment 351657 [details] [review] documents, selections: Exit selection only after printing has begun
Review of attachment 351657 [details] [review]: It's a bit unfortunate that we have to do this state bookkeeping, but solving that would require a larger refactor, so let's go for this now.