GNOME Bugzilla – Bug 748936
"Failed to Update" information dialog is too wide
Last modified: 2015-08-18 09:13:41 UTC
Created attachment 302916 [details] screenshot See the screenshot (this could be a different variation of bug 747148). Not only is the dialog window clearly the wrong size, but: 1. The dialog shouldn't be shown without a parent window. 2. The dialog text is scary, overly verbose, oddly formatted, and unnecessarily technical. 3. The title isn't formatted correctly (shouldn't the text be larger and in bold?) Suggested text for the dialog: "Failed to Update Oops! We're sorry: the update failed to install. Details: <output>" It would be good to show the output in monospace. Also, it would be good to respect line breaks in the output.
tests/testexpander.c in GTK+ shows how do an error dialog with details behind an expander
Created attachment 302947 [details] Proposed new look of the error message dialog with expander
Created attachment 302948 [details] [review] Initial patch introducing the expander This patch is a partial solution. It is backward compatible so it can be backported to 3.14 and 3.16 branch, as the bug 747148 has been. Here is what I have managed to change: * the title text is large and bold, * the detailed text is placed inside the GtkExpander, * the label of GtkExpander is "Details", not "Details:" to reuse the already existing string, avoid string freeze break and let this patch be backported, * the text label for the detailed text is wrapping, no more infinite length lines. This patch does not implement all features requested by Allan. As soon as the patch is reviewed, corrected, committed and backported I expect that all the missing features will be discussed and added. Please note that some changes (like new strings) cannot be backported.
Created attachment 302999 [details] [review] Initial patch introducing the expander (corrected) Sorry, I really hate this long functions and find them difficult to maintain. Here is the function gs_offline_updates_show_error() split into smaller functions: preparing the text messages separated from building the expander widget.
Review of attachment 302999 [details] [review]: Looks reasonable to me.
I do not consider this bug as resolved. The committed patch only fixes the problem partially and ensures the backward compatibility so it can be backported. Now we can make some bigger changes which will eventually land in 3.18 without worrying about the old branches. (In reply to Allan Day from comment #0) > [...] > 1. The dialog shouldn't be shown without a parent window. After a successful update there is a similar notification. The Review button opens the main g-s window, switches it to the Updates page and then shows a dialog box with a list of the recently applied updates. The dialog box is transient for the main application window. Let's make the error dialog follow the same pattern: open the main window on the Updates page + set it as a parent. > 2. The dialog text is scary, overly verbose, oddly formatted, and > unnecessarily technical. Please provide details how it should look, unless these details are already here. > 3. The title isn't formatted correctly (shouldn't the text be larger and in > bold?) Fixed with the latest patch: title is large and bold. > Suggested text for the dialog: > > "Failed to Update This is the title text, not changed, only the new formatting applied. > Oops! We're sorry: the update failed to install. This is the secondary text. The previous content was: "The offline update failed in an unexpected way." There are currently 9 messages which can be displayed here, you can find them near https://git.gnome.org/browse/gnome-software/tree/src/gs-offline-updates.c#n51, the list starts with "A previous update was unfinished.", then "Network access was required but not available." and so on. Please explain what you mean: should the text "Oops! We're sorry: the update failed to install." be always displayed here instead of those 9, or just instead of "The offline update failed in an unexpected way." or are you going to rework each of these messages individually? > Details: Now the string is "Details" (no colon at the end) because it does not break the string freeze in the old branches and can be backported. I have inserted an expander here and this is its label. Now it is a good moment to insert the missing colon. The old message dialog displayed the line "Detailed errors from the package manager follow:" here. I think it can be removed now. > <output>" Here is whatever is returned from pk_error_get_details() but it depends on the error code whether this text is displayed or not. If there is nothing to display then there is no expander here. > It would be good to show the output in monospace. Also, it would be good to > respect line breaks in the output. Let's see some screenshots or mockups with monospace font. And the line breaks have always been respected, as far as I know. The text in your screenshot had just one if its lines very long, now this issue has been fixed with wrapping.
Thanks for the extra details - I hadn't realised how these dialogs were being constructed. I'll provide mockups for this, but first some questions/comments about each of the error messages: * A previous update was unfinished. > Can't this be automatically corrected? If not, is there any advice we can offer to help resolve it? * Network access was required but not available. > Shouldn't this never happen with offline updates? How can an offline update even have access to the network? * An update was not signed in the correct way. > This is too technical for a user-facing explanation. Is there any advice we can offer to resolve the problem? * The update could not be completed. > Better to offer a generic response here, rather than having a custom error message that doesn't say anything useful. * The update was canceled. > How does a user manually cancel an offline update? I didn't think this was possible. * An offline update was requested but no packages required updating. > It sounds like this can only happen due to a lack of synchronisation between what is shown to the user and the actual state. Can't it be avoided? * No space was left on the drive. > Reword to be a bit more helpful? "No space was left on the drive. Please free up some space and try again." * An update failed to install correctly. > Better to go for a generic response here. * The offline update failed in an unexpected way. > Generic response here too. Is there any additional generic advice we could offer? What about "Please try to update again at another time." or "If this problem persists, please contact your operating system distributor for assistance."
These are the questions to Richard, I think. But if you want just to reword some messages you can do it yourself. By the way, when I was testing updates with not enough disk space I got the generic message "The offline update failed in an unexpected way." plus a detailed list of failed packages rather than "No space was left on the drive." which is not supposed to display details.
Created attachment 303185 [details] Screenshot of "failed to download" error after offline update The problem is, every once in a while it really does attempt to download packages during an offline update.
Created attachment 303300 [details] mockup See the attachment for a suggestion about how to display details in the dialog. I reviewed the error messages with Richard and Kalev yesterday. Here are the conclusions: * Generic message: "We're sorry: the update failed to install. Please wait for another update and try again. If the problem persists, contact your software provider." * "A previous update was unfinished." > "A previous update was unfinished." * "Network access was required but not available. > "Internet access was required but wasn't available. Please make sure that you have internet access and try again." * "An update was not signed in the correct way." > "There were security issues with the update. Please consult your software provider for more details." * "The update could not be completed." > "The update couldn't be installed; this is often a problem with the update itself. Please wait for another update and try again." * "An offline update was requested but no packages required updating." > "The system was already up to date." * "No space was left on the drive." > "There wasn't enough disk space. Please free up some space and try again." It would be better to give details for the amount of space needed: "123 MB disk space was needed, but only 23 MB was available. Please free up enough space and try again." * "An update failed to install correctly." > "The update couldn't be installed; this is often a problem with the update itself. Please wait for another update and try again." * "The offline update failed in an unexpected way." > Use the generic message.
(In reply to Allan Day from comment #10) > Created attachment 303300 [details] > mockup > > See the attachment for a suggestion about how to display details in the > dialog. 1. There is no expander. Is this intentional? Should we remove the expander and should the details be always visible (unless there is no details text)? 2. The details box in your mockup has white background (I guess: it has the same style as a text input box), in the patch it has grey background (the same style as a text label). Is this intentional? Do you want the white background, as every text input box? 3. In comment #0 you requested "Details:" (with the colon), in the mockup there is "Details" (no colon). Currently there is no colon but it is intended to be temporary and to ensure that it can be backported to the older versions. What do you choose for the final version? 4. The secondary text label looks like left aligned, currently it is centered. Shouldn't it be centered? What if the content is short and does not fill whole line? I think that centered would look better for short texts and not bad for long texts. 5. Should the whole dialog box be resizeable? In the latest patch it is resizeable when the expander is expanded, nonresizeable when it is not expanded or missing (thanks Matthias Clasen for this magic).
Created attachment 303556 [details] [review] Code cleanup in gs-application.c This patch is not directly related with this bug but will be helpful for the next patch which will make the main window appear as the transient parent of the "Failed to Update" message dialog.
Review of attachment 303556 [details] [review]: ok
Created attachment 303566 [details] [review] Don't show the dialog without a parent window Thanks for your review, Matthias. This patch implements Allan's request: > 1. The dialog shouldn't be shown without a parent window. Please note that: 1. This patch requires the previous one to be committed first. 2. Both of these patches look to be backportable to 3.16 and 3.14 but I don't insist on the backport. 3. This is not yet the end of the bug fix, we still need to reword the error messages as requested in comment #10 + waiting for Allan's response for comment #11. So please don't mark it as resolved/fixed.
(In reply to Rafal Luzynski from comment #11) > (In reply to Allan Day from comment #10) > > Created attachment 303300 [details] > > mockup > > > > See the attachment for a suggestion about how to display details in the > > dialog. > > 1. There is no expander. Is this intentional? Should we remove the expander > and should the details be always visible (unless there is no details text)? It is somewhat intentional - I personally don't see why hiding the details below an expander is useful. Most people will always expand to see what's in there, so what's the point? That said, I don't feel all that strongly about it - it's up to you. > 2. The details box in your mockup has white background (I guess: it has the > same style as a text input box), in the patch it has grey background (the > same style as a text label). Is this intentional? Do you want the white > background, as every text input box? The background helps to differentiate the "Details" heading from the content below. It also means that the layout won't have odd spacing when the details string is short. I don't have a particular preference for the colour though. > 3. In comment #0 you requested "Details:" (with the colon), in the mockup > there is "Details" (no colon). Currently there is no colon but it is > intended to be temporary and to ensure that it can be backported to the > older versions. What do you choose for the final version? I prefer it without the colon, but again I'm not going to strongly push for this. > 4. The secondary text label looks like left aligned, currently it is > centered. Shouldn't it be centered? What if the content is short and does > not fill whole line? I think that centered would look better for short texts > and not bad for long texts. You are right that it should be centered - this is determined by GTK, so we should just follow that. > 5. Should the whole dialog box be resizeable? In the latest patch it is > resizeable when the expander is expanded, nonresizeable when it is not > expanded or missing (thanks Matthias Clasen for this magic). My instinct would be to try without resizing and go from there. If the details box is scrollable, it shouldn't be necessary, should it?
Thank you for your responses, Allan. I'm going to implement as much as possible of your suggestions. But I'd like to comment on this one: (In reply to Allan Day from comment #15) > (In reply to Rafal Luzynski from comment #11) > > 5. Should the whole dialog box be resizeable? In the latest patch it is > > resizeable when the expander is expanded, nonresizeable when it is not > > expanded or missing (thanks Matthias Clasen for this magic). > > My instinct would be to try without resizing and go from there. If the > details box is scrollable, it shouldn't be necessary, should it? Some users may prefer resizing over scrolling. This depends on the amount of text, screen size, personal preferences etc. On the other hand, those who don't want resizing may not even be aware that the dialog box is resizeable, see bug 736174 comment 4. So I'm going to set the dialog box resizeable if there are details and nonresizeable if there are no details.
Created attachment 303718 [details] Proposed new look of the error message dialog without an expander This is the new look of the dialog with a minimum effort: most settings are default. Is this what you will like?
Created attachment 305017 [details] Proposed new look of the error message dialog without an expander (2nd version) This is how it will look. I hope that everyone will be satisfied.
Created attachment 305020 [details] [review] GtkTextView instead of GtkExpander, according to the mockup And this is the implementation of Allan's design. Please note that this patch still seems to be backportable so please consider backporting to 3.16 and maybe even 3.14. This is probably the last backportable patch. Also note that there are another two patches which must be committed before this one. Again, this is not the end of this bug. What we still need: 1. Reword the error messages as requested in comment #10. 2. Maybe remove the tricky workaround and use the correct API or CSS (requires bug 406159 to be fixed.)
Thanks for working on these bugs, Rafal!
Created attachment 305128 [details] [review] Error messages reworded, as suggested in comment #10 Please note that this patch changes some string so it is NOT backportable. What next? Should we keep this bug open until bug 406159 is fixed so we can provide a better solution for the margins? My suggestion is to keep it open until the end of this or maybe the next month and then if bug 406159 is not fixed then we'll have to assume it won't be fixed soon and this tricky solution from comment #19 will have to be permanent for some longer time.
Created attachment 309436 [details] [review] Since bug 406159 has been fixed, use CSS styling instead of a workaround This should close the issue. Note that this fix will require GTK 3.18 so it cannot be backported to the old branches.
Review of attachment 309436 [details] [review]: Looks good to me, thanks! Might be worth adding a dependency on a recent enough gtk+ release in configure.ac.
Pushed with the configure.ac change to bump gtk+ dep to 3.17.7.