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 759248 - preview: Fix possible warnings exiting fullscreen
preview: Fix possible warnings exiting fullscreen
Status: RESOLVED FIXED
Product: gnome-documents
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: GNOME documents maintainer(s)
GNOME documents maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2015-12-09 13:41 UTC by Bastien Nocera
Modified: 2016-01-04 00:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
preview: Fix possible warnings exiting fullscreen (1.08 KB, patch)
2015-12-09 13:41 UTC, Bastien Nocera
none Details | Review
preview: Fix possible warnings exiting fullscreen (1.33 KB, patch)
2015-12-10 10:22 UTC, Bastien Nocera
none Details | Review
preview: Fix possible warnings exiting fullscreen (1.08 KB, patch)
2015-12-11 10:53 UTC, Bastien Nocera
committed Details | Review
preview: Check if this._jobFind is null (971 bytes, patch)
2016-01-03 13:23 UTC, Alessandro Bono
committed Details | Review

Description Bastien Nocera 2015-12-09 13:41:51 UTC
Reproducer in the commit description.
Comment 1 Bastien Nocera 2015-12-09 13:41:56 UTC
Created attachment 317024 [details] [review]
preview: Fix possible warnings exiting fullscreen

After opening a document, turning on the fullscreen mode, and starting a
search (type a few letters), you'll get warnings from EvView when "Esc"
is pressed.

This is caused by the search being reset when exiting the preview. As we
don't have a document, or document model anymore, there's no search to
be done.
Comment 2 Cosimo Cecchi 2015-12-09 16:20:51 UTC
Review of attachment 317024 [details] [review]:

Wouldn't it be possible to avoid the 'updated' signal to be emitted at all (i.e. cancel/destroy the EvJobFind) when the model is unset?
Comment 3 Bastien Nocera 2015-12-10 10:22:44 UTC
Created attachment 317094 [details] [review]
preview: Fix possible warnings exiting fullscreen

After opening a document, turning on the fullscreen mode, and starting a
search (type a few letters), you'll get warnings from EvView when "Esc"
is pressed.

This is caused by the search being reset when exiting the preview. As we
don't have a document, or document model anymore, there's no search to
be done.
Comment 4 Bastien Nocera 2015-12-10 10:23:39 UTC
(In reply to Cosimo Cecchi from comment #2)
> Review of attachment 317024 [details] [review] [review]:
> 
> Wouldn't it be possible to avoid the 'updated' signal to be emitted at all
> (i.e. cancel/destroy the EvJobFind) when the model is unset?

Good call, I did that when the model is being reset.
Comment 5 Cosimo Cecchi 2015-12-10 19:09:17 UTC
Review of attachment 317094 [details] [review]:

Thanks, looks good, but it also contains an update to the libgd submodule which shouldn't be here AFAICT.
Comment 6 Bastien Nocera 2015-12-11 10:53:22 UTC
Created attachment 317198 [details] [review]
preview: Fix possible warnings exiting fullscreen

After opening a document, turning on the fullscreen mode, and starting a
search (type a few letters), you'll get warnings from EvView when "Esc"
is pressed.

This is caused by the search being reset when exiting the preview. As we
don't have a document, or document model anymore, there's no search to
be done.
Comment 7 Bastien Nocera 2015-12-11 10:54:48 UTC
Attachment 317198 [details] pushed as cd6d7c7 - preview: Fix possible warnings exiting fullscreen
Comment 8 Alessandro Bono 2016-01-03 13:22:49 UTC
If you open a document and go back without searching, it complains about a null reference.
(org.gnome.Documents:29808): Gjs-WARNING **: JS ERROR: Exception in callback for signal: window-mode-changed: TypeError: this._jobFind is null
PreviewView<.setModel@resource:///org/gnome/Documents/js/preview.js:572
Comment 9 Alessandro Bono 2016-01-03 13:23:49 UTC
Created attachment 318180 [details] [review]
preview: Check if this._jobFind is null

Fallout from cd6d7c76ddf063fe0f285044c06ced0ce45e97d7
Comment 10 Cosimo Cecchi 2016-01-03 18:35:15 UTC
Review of attachment 318180 [details] [review]:

Looks good, thanks.
Comment 11 Alessandro Bono 2016-01-04 00:48:43 UTC
Attachment 318180 [details] pushed as f5b2047 - preview: Check if this._jobFind is null