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 734603 - Use re-read action state changes to drive UI changes
Use re-read action state changes to drive UI changes
Status: RESOLVED FIXED
Product: sound-juicer
Classification: Applications
Component: interface
git master
Other Linux
: Normal enhancement
: ---
Assigned To: Sound Juicer Maintainers
Sound Juicer Maintainers
Depends on: 705159 724281 736492
Blocks:
 
 
Reported: 2014-08-11 08:54 UTC by Phillip Wood
Modified: 2015-02-16 18:28 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Reformat metadata_cb (811 bytes, patch)
2014-08-11 12:05 UTC, Phillip Wood
none Details | Review
Use re-read state for UI. (5.64 KB, patch)
2014-08-11 12:05 UTC, Phillip Wood
none Details | Review
Don't push endless status bar messages. (3.82 KB, patch)
2014-08-11 12:06 UTC, Phillip Wood
none Details | Review
Use g_application_mark_busy() (1.54 KB, patch)
2014-09-05 10:03 UTC, Phillip Wood
none Details | Review
Don't push endless status bar messages. (3.59 KB, patch)
2014-09-05 10:04 UTC, Phillip Wood
none Details | Review
Use g_application_mark_busy() (3.40 KB, patch)
2014-09-22 13:19 UTC, Phillip Wood
none Details | Review
Use re-read state for UI. (3.46 KB, patch)
2014-09-22 13:19 UTC, Phillip Wood
none Details | Review
Don't push endless status bar messages. (3.66 KB, patch)
2014-09-22 13:19 UTC, Phillip Wood
none Details | Review

Description Phillip Wood 2014-08-11 08:54:07 UTC
Now that the re-read action has state, use this change the status bar and mouse cursor rather than changing them directly everywhere the state of the action is changed. This means we don't have to remember to change the UI when we change the state, it just happens automatically.
Comment 1 Phillip Wood 2014-08-11 12:05:44 UTC
Created attachment 283083 [details] [review]
Reformat metadata_cb
Comment 2 Phillip Wood 2014-08-11 12:05:54 UTC
Created attachment 283084 [details] [review]
Use re-read state for UI.

Use the state of the re-read action to change the status bar and mouse
cursor rather than changing them directly everywhere the state of the
action is changed. This means we don't have to remember to change the
UI when we change the state, it just happens automatically.
Comment 3 Phillip Wood 2014-08-11 12:06:36 UTC
Created attachment 283085 [details] [review]
Don't push endless status bar messages.

When extracting and playing the status bar is continuously updated
with gtk_statusbar_push but the messages are never cleared. To clear
the status bar an empty message is pushed. This interacts badly with
the re-read code that pushes and then pops messages and also wastes
memory. Fix this by (i) using different context ids for extracting,
playing & re-reading. (ii) using gtk_statusbar_remove_all rather than
pushing empty messages to clear the statusbar.

This exposes GTK bug 724281
Comment 4 Phillip Wood 2014-09-04 10:13:35 UTC
I wonder if we should use g_application_mark_busy() rather than changing the mouse cursor. It would be nice to get this sorted for 3.14 as at the moment there is no busy indication while the metadata is being retrieved. This is because reread_cd() is now only called once, before the main window is realized.
Comment 5 Bastien Nocera 2014-09-04 10:14:41 UTC
"mark busy" is great for network downloads.
Comment 6 Phillip Wood 2014-09-05 10:03:40 UTC
Created attachment 285465 [details] [review]
Use g_application_mark_busy()

Marking the application as busy is a better option than changing the
mouse cursor. We have to wait for the window to be realized before
calling g_application_mark_busy() otherwise the busy spinner doesn't
appear in gnome-shell
Comment 7 Phillip Wood 2014-09-05 10:04:53 UTC
Created attachment 285466 [details] [review]
Don't push endless status bar messages.

This is the same patch as before rebased onto 
"Use g_application_mark_busy()"


When extracting and playing the status bar is continuously updated
with gtk_statusbar_push but the messages are never cleared. To clear
the status bar an empty message is pushed. This interacts badly with
the re-read code that pushes and then pops messages and also wastes
memory. Fix this by (i) using different context ids for extracting,
playing & re-reading. (ii) using gtk_statusbar_remove_all rather than
pushing empty messages to clear the statusbar.
Comment 8 Phillip Wood 2014-09-10 10:39:55 UTC
(In reply to comment #6)
> We have to wait for the window to be realized before
> calling g_application_mark_busy() otherwise the busy spinner doesn't
> appear in gnome-shell

That's not true, I've just tested it again and most times the spinner still isn't showing even if g_application_mark_busy() is called once the window is realized. I'm not sure if it's a problem with the code, my installation of gnome-shell or a bug in gnome-shell. I'm not sure what the expected behaviour of the shell is. Help!
Comment 9 Phillip Wood 2014-09-11 17:31:30 UTC
It looks like there is a race between sound-juicer changing it's busy status and gnome-shell subscribing to PropertiesChanged so it misses the change in the busy status. I've filed bug 736492 against gnome-shell.
Comment 10 Phillip Wood 2014-09-22 13:19:24 UTC
Created attachment 286807 [details] [review]
Use g_application_mark_busy()

The problem with the spinner not showing up is a bug in
gnome-shell. This is a simplified version of the previous patch.

Marking the application as busy is a better option than changing the
mouse cursor. This exposes bug 736492 in gnome-shell.
Comment 11 Phillip Wood 2014-09-22 13:19:40 UTC
Created attachment 286808 [details] [review]
Use re-read state for UI.

Use the state of the re-read action to change the status bar and mouse
cursor rather than changing them directly everywhere the state of the
action is changed. This means we don't have to remember to change the
UI when we change the state, it just happens automatically.
Comment 12 Phillip Wood 2014-09-22 13:19:49 UTC
Created attachment 286809 [details] [review]
Don't push endless status bar messages.

When extracting and playing the status bar is continuously updated
with gtk_statusbar_push but the messages are never cleared. To clear
the status bar an empty message is pushed. This interacts badly with
the re-read code that pushes and then pops messages and also wastes
memory. Fix this by (i) using different context ids for extracting,
playing & re-reading. (ii) using gtk_statusbar_remove_all rather than
pushing empty messages to clear the statusbar.

This exposes bug 724281 in Gtk+
Comment 13 Phillip Wood 2015-02-16 18:28:26 UTC
Attachment 286807 [details] pushed as 976572e - Use g_application_mark_busy()
Attachment 286808 [details] pushed as 6c7c530 - Use re-read state for UI.
Attachment 286809 [details] pushed as 113d2b1 - Don't push endless status bar messages.