GNOME Bugzilla – Bug 584530
Progress info window should provide progress in a more streamlined way
Last modified: 2015-11-23 18:17:38 UTC
The new progress info window states "File Operations" as its title and puts a static icon in the notification area. When there are long running operations and this window is minimized or (more likely) covered with other windows there's no quick way for the user to know the progress. A simple way to report progress could be updating the window title with a percentage of the total progress at the beggining like the Downloads window in Firefox does so that glancing at the window list provides a quick overview of progress. The old code also used a progress icon which could be brought back and also used in the notification area icon. Other information:
Created attachment 135757 [details] [review] update the progress window title with total progress percentage This is a preliminary patch to put the progress in the title. I have one doubt: I didn't use any locking to access the objects in the active_progress_infos GList but I see locking being used in some places. Given the way that update_progress_window_title() is called does it need locking? It did work for me though.
You need locking when accessing active_progress_infos. The easiest way to do that is to use nautilus_get_all_progress_info() which does the locking and refs the infos. (just make sure you unref them when done.) Also, I think we should special case the n_valid_progress_ops == 1 case to get a better string for that (i.e. not "30% of 1", just "30%").
Created attachment 135854 [details] [review] new patch Ok, so the list itself is the contention point. I'm using the locked list copy now. I've also factored the progress string construction and thus am now updating the GtkStatusIcon's tooltip along with the window title.
Do you think this needs more work?
Comment on attachment 135854 [details] [review] new patch Hi, thanks for the patch and sorry for the late review. >+ if (n_ops > 1) { >+ return g_strdup_printf (_("%d%% of %'d - File Operations"), >+ (int) (progress * 100), n_ops); >+ if (n_ops == 1) { >+ return g_strdup_printf (_("%d%% - File Operations"), >+ (int) (progress * 100)); You should probably add a comment for translators above these lines explaining the meaning of "%d%% of %'d" strings (something like "Translators: the first %d is the global completed percentage of the file operations currently running, the second %d is the number of file operations currently running."). Also I don't like the wording of these new strings: in case we have more than one operation running, I'd use something like "%d%% of %'d tasks - File Operations", when there's only one "%d%% completed - File Operations". >- gtk_window_set_title (GTK_WINDOW (progress_window), >- _("File Operations")); >+ window_title = create_progress_string_for_n_ops (0.0, 0); >+ gtk_window_set_title (GTK_WINDOW (progress_window), window_title); >+ g_free (window_title); No need for allocating |window_title| here, just use gtk_window_set_title (w, _("File Operations")). >+ if (n_progress_ops == 0) { >+ gtk_status_icon_set_visible (status_icon, FALSE); >+ gtk_widget_hide (get_progress_window ()); >+ goto set_strings; Why do you still need to set strings on both the progress window and the status icon if we just hide them? I don't particularly like that you use the same text for both the status icon tooltip and the window title, as it makes sense for the tooltip to use a different capitalization. Something like "%d%% of %'d active file operations" (with a singular version as well) should be better.
Created attachment 145318 [details] [review] new patch Ok, agreed. I was trying really hard to do DRY that day... Now I tried to follow what Firefox does more closely. Mimicking its status bar download message on the status icon tooltip text. Tell me what you think, thanks!
*** Bug 590071 has been marked as a duplicate of this bug. ***
Comment on attachment 145318 [details] [review] new patch $:andre\> patch --no-backup-if-mismatch -p1 < patch patching file libnautilus-private/nautilus-progress-info.c Hunk #1 FAILED at 204. Hunk #2 succeeded at 486 with fuzz 2 (offset 236 lines). Hunk #3 FAILED at 339. 2 out of 3 hunks FAILED -- saving rejects to file libnautilus-private/nautilus-progress-info.c.rej Hence setting 'needs-rework' as patch does not apply cleanly.
The window in question is no more with 3.18, closing as OBSOLETE.