This is a spin off of bug 342350.

Requesting blocking Firefox 2.0. While cleaning up this code I've found a couple of bugs with this wizard and I should have a patch ready soon.

I still need to verify this does the right thing in regards to hashes.

beltzner, could you take a look at the screenshots? If there are more string changes needed I'd prefer to fix them in another bug so I can get this landed when it is ready.

beltzner, for starters the word restart is no longer necessary since we don't display any other app ui and perform the restart no matter what when the wizard is closed.

--> blocking Fx2b2; I'd like to get it into Fx2b1 if possible, though, but that'll be depending on getting a review in time and making an assertion of risk. (string comments coming)

beltzner, so, the error details window is not used but it should be. We currently just go on happily when an update error happens instead of informing the user that the install failed. The old strings for this were: installErrorDescription=The following components could not be installed due to errors (the file could not be downloaded, was corrupt, or for some other reason). checkingErrorDescription=%S could not check for updates to the following components (either the update server(s) did not respond, or the update service(s) were not found). They are displayed on top of a list and I'll try to get screenshots submitted shortly.

beltzner, this is pretty damn awful IMO. This is the error dialog that needs those two strings in my previous comment... I'm going to try to clean up this ui some but it should give you an idea of where it is used.

Benjamin, I highly suspect you won't much like this patch. All I can say is the original code is full of cruft, dead end paths, etc. and this should at least make it easier to work with by removing said cruft, etc. while showing the messages that were broken since the app update code was removed from this code.

beltzner, this should be wide enough to display the errors we might have. It is still sucky but this should be less sucky. btw: I made it the same width as the wizard that opens it.

Comment on attachment 228222 [details] [diff] [review] patch - forgot a file in the last one Some comments to hopefully help with the review >Index: toolkit/mozapps/update/content/errors.xul ... > <dialog id="errors" > xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" > title="&errors.title;" > onload="init()" >- style="width: 28em;" >+ style="width: 40em;" We weren't displaying the entire error message due to the width being too small. >@@ -59,17 +59,17 @@ ... >- var str = updateStrings.getFormattedString(state + "ErrorDescription", [brandShortName]); >+ var str = updateStrings.getFormattedString(state + "ErrorDesc", [brandShortName]); l10n change >@@ -99,14 +99,14 @@ ... >- <separator/> >+ <separator class="thin"/> > <listbox id="extensions" rows="7"/> >- <separator/> >+ <separator class="thin"/> There was too much space between the text and the list (see screenshot). >Index: toolkit/mozapps/extensions/content/update.js ... >@@ -15,95 +15,85 @@ >-// >-// This UI can be opened from the following places, and in the following modes: >-// >-// - from the Version Compatibility Checker at startup >-// as the application starts a check is done to see if the application being >-// started is the same version that was started last time. If it isn't, a >-// list of UpdateItems that are incompatible with the verison being >-// started is generated and this UI opened with that list. This UI then >-// offers to check for updates to those UpdateItems that are compatible >-// with the version of the application being started. >-// >-// In this mode, the wizard is opened to panel 'mismatch'. >+// This UI is only opened from the Extension Manager when the app is upgraded. The old comment was bogus. >-const PREF_UPDATE_EXTENSIONS_AUTOUPDATEENABLED = "extensions.update.autoUpdateEnabled"; I don't think this pref was ever implemented... it isn't used now at least. > >-var gShowMismatch = null; This was initialized to null and never changed though it was checked in if statements. > var gUpdateWizard = { >- // The items to check for updates for (e.g. an extension, some subset of extensions, >- // all extensions, a list of compatible extensions, etc...) >+ // When synchronizing app compatibility info this contains all installed >+ // add-ons. When checking for compatible versions this contains only >+ // incompatible add-ons. comment cleanup. > items: [], > // The items that we found updates available for > itemsToUpdate: [], >- // The items that we successfully installed updates for >- updatedCount: 0, not used anywhere > shouldSuggestAutoChecking: false, > shouldAutoCheck: false, > xpinstallEnabled: false, > xpinstallLocked: false, >- remainingExtensionUpdateCount: 0, not used anywhere >- this.shouldSuggestAutoChecking = >- gShowMismatch && >- !pref.getBoolPref(PREF_UPDATE_EXTENSIONS_AUTOUPDATEENABLED); >+ >+ try { >+ this.shouldSuggestAutoChecking = >+ !pref.getBoolPref(PREF_UPDATE_EXTENSIONS_ENABLED); >+ } >+ catch (e) { >+ } Mainly to protect XULRunner apps since other apps specify this. >- pref.setBoolPref(PREF_UPDATE_EXTENSIONS_ENABLED, this.shouldAutoCheck); >+ pref.setBoolPref(PREF_UPDATE_EXTENSIONS_ENABLED, this.shouldAutoCheck); sorry, whitespace cleanup. >@@ -129,28 +119,30 @@ ... >+ // Displays a list of items that had an error during the update check. We >+ // don't display the actual error that occured since >+ // nsIAddonUpdateCheckListener doesn't return the error details. We don't return actual errors regretfully so all we have here is success and failure when checking an update rdf. Misspelled occurred and fixed locally. ... >- errors.push({ name: this.errorItems[i].name, error: true, >- item: this.errorItems[i] }); >+ errors.push({ name: this.errorItems[i].name, error: true }); The nsIUpdateItem was never used in errors.xul ... >- if (this.errorOnGeneric || this.errorItems.length > 0) >+ if (this.errorItems.length > 0) this.errorOnGeneric is not decalred or set anywhere and has been around since 1.0. >@@ -248,34 +240,36 @@ ... > var gUpdatePage = { >+ _totalCount: 0, Added _totalCount to gUpdatePage for consistency with the other wizard pages. > var gFoundPage = { >- _nonAppItems: [], >- >- _newestInfo: null, No longer needed and they weren't referenced anywhere. >- buildAddons: function () It is not possible to init this page more than once so there is no need for the buildAddons function. >+ onPageShow: function () > { >- var hasExtensions = false; No longer needed since this code no longer handles app update. >- var foundAddonsList = document.getElementById("found.addons.list"); >- var uri = Components.classes["@mozilla.org/network/standard-url;1"] >- .createInstance(Components.interfaces.nsIURI); >+ gUpdateWizard.setButtonLabels(null, true, >+ "installButtonText", false, >+ null, false); >+ >+ var foundUpdates = document.getElementById("found.updates"); > var itemCount = gUpdateWizard.itemsToUpdate.length; > for (var i = 0; i < itemCount; ++i) { > var item = gUpdateWizard.itemsToUpdate[i]; >- var checkbox = document.createElement("checkbox"); >- foundAddonsList.appendChild(checkbox); >- checkbox.setAttribute("type", "update"); >- checkbox.label = item.name + " " + item.version; >- checkbox.setAttribute("URL", item.xpiURL); >- checkbox.setAttribute("hash", item.xpiHash); >- checkbox.infoURL = ""; >- checkbox.internalName = ""; >- uri.spec = item.xpiURL; >- checkbox.setAttribute("source", uri.host); >- checkbox.checked = true; >- hasExtensions = true; >+ var listItem = foundUpdates.appendItem(item.name + " " + item.version, >+ item.xpiURL); >+ listItem.setAttribute("type", "checkbox"); >+ listItem.setAttribute("checked", "true"); >+ listItem.setAttribute("URL", item.xpiURL); >+ listItem.setAttribute("hash", item.xpiHash); The wizard now uses a listbox. >- if (hasExtensions) { >- var addonsHeader = document.getElementById("addons"); >- var strings = document.getElementById("updateStrings"); >- addonsHeader.label = strings.getFormattedString("updateTypeAddons", [itemCount]); >- addonsHeader.collapsed = false; >- } This was needed to select the radio for extensions when we had app update, etc. included in this code and this is no longer needed. >- _initialized: false, It is not possible to init this page more than once so there is no need to know it has been initialized. ... > if (!gUpdateWizard.xpinstallEnabled) { >- document.documentElement.getButton("next").disabled = true; handled by the call to updateNextButton. >+ else { >+ //XXXrstrong - this string should be set in update.xul from update.dtd >+ // for Gecko Version 1.9. >+ var strings = document.getElementById("updateStrings"); >+ var text = strings.getString("foundInstructions"); >+ var foundInstructions = document.getElementById("foundInstructions"); >+ foundInstructions.hidden = false; >+ foundInstructions.appendChild(document.createTextNode(text)); This really should be done in the dtd. Since there are string changes should it be done now? >+ var foundUpdates = document.getElementById("found.updates"); >+ var updates = foundUpdates.getElementsByTagName("listitem");; >+ for (var i = 0; i < updates.length; ++i) { >+ if (!updates[i].checked) >+ continue; >+ oneChecked = true; >+ break; > } Changed to support the listbox > > var gInstallingPage = { > _installing : false, >- _restartRequired : false, No longer needed and we always restart anyways. >- var updates = document.getElementById("found.updates"); >- var checkboxes = updates.selectedItem.getElementsByTagName("checkbox"); >- for (var i = 0; i < checkboxes.length; ++i) { >- if (checkboxes[i].type == "update" && checkboxes[i].checked) { >- items.push(checkboxes[i].URL); >- hashes.push(checkboxes[i].hash ? checkboxes[i].hash : null); >- this._objs.push({ name: checkboxes[i].label }); >- } >+ var foundUpdates = document.getElementById("found.updates"); >+ var updates = foundUpdates.getElementsByTagName("listitem");; >+ for (var i = 0; i < updates.length; ++i) { >+ if (!updates[i].checked) >+ continue; >+ items.push(updates[i].value); >+ hashes.push(updates[i].getAttribute("hash") ? updates[i].getAttribute("hash") : null); >+ this._objs.push({ name: updates[i].label }); > } Changed to support the listbox >@@ -475,95 +431,87 @@ > case nsIXPIProgressDialog.INSTALL_START: > var label = strings.getFormattedString("installingPrefix", [this._objs[aIndex].name]); > var actionItem = document.getElementById("actionItem"); > actionItem.value = label; > this._installing = true; > break; > case nsIXPIProgressDialog.INSTALL_DONE: > switch (aValue) { >- case 999: >- this._restartRequired = true; >- break; >- case 0: >- --gUpdateWizard.remainingExtensionUpdateCount; Set but not used anywhere >+ case 999: >+ case 0: > break; >+ default: >+ this._errors.push({ name: this._objs[aIndex].name, error: aValue }); > } So we can display install errors > >- _objs: [], declared twice >- _errors: false, Now it just uses the _errors array > var gErrorsPage = { > onPageShow: function () > { > document.documentElement.getButton("finish").focus(); > }, > > onShowErrors: function () > { >- gUpdateWizard.showErrors("install", gInstallingPage._objs); >+ gUpdateWizard.showErrors("install", gInstallingPage._errors); gInstallingPage._objs never contained errors so installs were always shown as successful and errors were never shown. > var gFinishedPage = { > onPageShow: function () > { > gUpdateWizard.setButtonLabels(null, true, null, true, null, true); > document.documentElement.getButton("finish").focus(); > >- if (gUpdateWizard.xpinstallLocked) >- { >+ if (gUpdateWizard.xpinstallLocked) { > document.getElementById("adminDisabled").hidden = false; > document.getElementById("incompatibleRemainingLocked").hidden = false; >+ document.getElementById("finishedMismatch").hidden = false; >+ document.getElementById("incompatibleAlert").hidden = false; > } > else { > document.getElementById("updated").hidden = false; > document.getElementById("incompatibleRemaining").hidden = false; > if (gUpdateWizard.shouldSuggestAutoChecking) { >+ document.getElementById("finishedEnableCheckingDesc").hidden = false; > document.getElementById("finishedEnableChecking").hidden = false; > document.getElementById("finishedEnableChecking").click(); > } > } >- >- if (gShowMismatch || gUpdateWizard.xpinstallLocked) { >- document.getElementById("finishedMismatch").hidden = false; >- document.getElementById("incompatibleAlert").hidden = false; >- } xpinstallLocked is handled above and gShowMismatch is always null. > var gNoUpdatesPage = { > onPageShow: function (aEvent) > { > gUpdateWizard.setButtonLabels(null, true, null, true, null, true); > document.documentElement.getButton("finish").focus(); >- if (gShowMismatch) { >- document.getElementById("introUser").hidden = true; >- document.getElementById("introMismatch").hidden = false; >- document.getElementById("mismatchNoUpdates").hidden = false; >+ document.getElementById("introUser").hidden = true; >+ document.getElementById("introMismatch").hidden = false; >+ document.getElementById("mismatchNoUpdates").hidden = false; gShowMismatch is always null so there was missing ui. >- if (gUpdateWizard.shouldSuggestAutoChecking) { >- document.getElementById("mismatchIncompatibleRemaining").hidden = true; >- document.getElementById("mismatchIncompatibleRemaining2").hidden = false; >- document.getElementById("mismatchFinishedEnableChecking").hidden = false; >- } >+ if (gUpdateWizard.shouldSuggestAutoChecking) { >+ document.getElementById("mismatchIncompatibleRemaining").hidden = true; >+ document.getElementById("mismatchIncompatibleRemaining2").hidden = false; >+ document.getElementById("mismatchFinishedEnableChecking").hidden = false; >+ document.getElementById("mismatchFinishedEnableChecking").click(); Sorry, more whitespace. >Index: toolkit/mozapps/extensions/content/update.xul ... >@@ -33,34 +33,32 @@ > # decision by deleting the provisions above and replace them with the notice > # and other provisions required by the GPL or the LGPL. If you do not delete > # the provisions above, a recipient may use your version of this file under > # the terms of any one of the MPL, the GPL or the LGPL. > # > # ***** END LICENSE BLOCK ***** > > <?xml-stylesheet href="chrome://global/skin/" type="text/css"?> >-<?xml-stylesheet href="chrome://mozapps/content/extensions/update.css" type="text/css"?> no longer needed > <wizard id="updateWizard" > xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" > title="&updateWizard.title;" > windowtype="Update:Wizard" > onload="gUpdateWizard.init();" > onwizardfinish="gUpdateWizard.onWizardFinish();" > onclose="return gUpdateWizard.onWizardClose(event);" >- style="width: 47em; min-height: 35em;" Use the default dimensions for a wizard

String comments for the screenshots in attachment 228118 [details] (as read top-to-bottom, left-to-right) cc'ing reed for orthography: screen 1: panel title: "Checking Add-On Compatibility" progress text: "Checking your add-ons to see if they are compatible&#133;" screen 2: panel title: "Found Incompatible Add-Ons" text: "The following add-ons are not compatible with this version of %S and have been disabled:" text: "%S can check to see if updates are available to make these add-ons compatible with this version." (NOTE: shouldn't we be doing the xpinstall check *before* we bother to check for available updates?) screen 3: panel title: "Checking for Compatibility Updates" text: "Checking for available updates to incompatible add-ons ..." - just need the text below the progress indicator, remove the stuff above screen 4: panel title: "Unable to Install Updates" text: "There are updates available, but they can't be installed because software installation for Minefield has been disabled. Please contact your system administrator for assistance." - the "don't check" button shouldn't be here - the "Finish" button should be at the right, selected by default screen 5: panel title: "Found Compatibility Updates" text: "You may select the updates you would like %S to install." screen 6: panel title: "Found Compatibility Updates" text: "The selected updates can't be installed because software installation is currently disabled. You can change this setting below." screen 7: panel title: "Installing Compatibility Updates" text: "Downloading and installing updates to your add-ons" progress indication text is good screen 8: panel title: "Compatibility Updates Installed" text: "%S has installed the updates to your add-ons." screen 9: panel title: "No Compatibility Updates Found" text: "%S was unable to find any updates for your add-ons to make them compatible with this version. These updates may be available in the future. %S will check periodically and inform you when compatible versions of your add-ons are found." screen 10: panel title: "Some Compatibility Updates Found" text: "%S was unable to find any updates for some of your add-ons to make them compatible with this version. These updates may be available in the future. %S can check periodically and inform you when compatible versions of your add-ons are found." - checkText is good as is screen 11: panel title: "No Compatibility Updates Found" text: "%S was unable to find any updates for your add-ons to make them compatible with this version. These updates may be available in the future. %S can check periodically and inform you when compatible versions of your add-ons are found." - checkText is good as is

Thank you beltzner! (In reply to comment #13) ... > screen 1: > panel title: "Checking Add-On Compatibility" We've consistently used Add-on and Add-ons when it should be capitalized ... > screen 2: > panel title: "Found Incompatible Add-Ons" Same re: Add-ons > text: "The following add-ons are not compatible with this version of %S and > have been disabled:" > text: "%S can check to see if updates are available to make these add-ons > compatible with this version." The sentence structure seems a bit awkward - "can check to see". I still don't like it but something along the lines of: text: "%S can check for compatible versions of these add-ons." > (NOTE: shouldn't we be doing the xpinstall check *before* we bother to check > for available updates?) The reason it is done at this point is if no updates are found then this ui doesn't need to ask the user to enable xpinstall. > screen 3: > panel title: "Checking for Compatibility Updates" > text: "Checking for available updates to incompatible add-ons ..." No space between "add-ons" and "..." right? > - just need the text below the progress indicator, remove the stuff above The text will display the add-on name that is being checked similar to screen 7. I just caught it when it was first starting. Do you still want it removed? > screen 4: > panel title: "Unable to Install Updates" > text: "There are updates available, but they can't be installed because > software installation for Minefield has been disabled. Please contact your > system administrator for assistance." > - the "don't check" button shouldn't be here > - the "Finish" button should be at the right, selected by default Would it be ok if the "don't check" button was a "cancel" button and disabled as it is elsewhere? > screen 5: > panel title: "Found Compatibility Updates" > text: "You may select the updates you would like %S to install." > > screen 6: > panel title: "Found Compatibility Updates" > text: "The selected updates can't be installed because software installation > is currently disabled. You can change this setting below." For screen 5 and 6 - I had problems with the existing ui since in both it is possible to check / uncheck add-ons to update but only in one is it stated "You may select the updates you would like %S to install." > screen 7: > panel title: "Installing Compatibility Updates" > text: "Downloading and installing updates to your add-ons" > progress indication text is good > > screen 8: > panel title: "Compatibility Updates Installed" > text: "%S has installed the updates to your add-ons." > > screen 9: > panel title: "No Compatibility Updates Found" > text: "%S was unable to find any updates for your add-ons to make them > compatible with this version. These updates may be available in the future. %S > will check periodically and inform you when compatible versions of your add-ons > are found." "These updates may be available in the future." seems a bit superfluous in relation to the sentence that follows it. > screen 10: > panel title: "Some Compatibility Updates Found" > text: "%S was unable to find any updates for some of your add-ons to make > them compatible with this version. These updates may be available in the > future. %S can check periodically and inform you when compatible versions of > your add-ons are found." Same here > screen 11: > panel title: "No Compatibility Updates Found" > text: "%S was unable to find any updates for your add-ons to make them > compatible with this version. These updates may be available in the future. %S > can check periodically and inform you when compatible versions of your add-ons > are found." Same here

beltzner, from comment #7 there are a couple of more. installErrorDescription=The following components could not be installed due to errors (the file could not be downloaded, was corrupt, or for some other reason). checkingErrorDescription=%S could not check for updates to the following components (either the update server(s) did not respond, or the update service(s) were not found). See attachment #228230 [details] for context

(In reply to comment #13) > String comments for the screenshots in attachment 228118 [details] [edit] (as read > top-to-bottom, left-to-right) cc'ing reed for orthography: Glad to help. Ask me any time. :) > screen 1: > panel title: "Checking Add-On Compatibility" Why "Add-On" in title and "add-ons" in text (difference in plurality)? I'd go with "Add-Ons" in title, too. > progress text: "Checking your add-ons to see if they are compatible&#133;" Compatible with what? Maybe "compatible with this version"? > screen 3: > panel title: "Checking for Compatibility Updates" > text: "Checking for available updates to incompatible add-ons ..." > - just need the text below the progress indicator, remove the stuff above Remove the space between "add-ons" and "...". Also, shouldn't you be using "&#133;" instead of "..." here? > screen 4: > panel title: "Unable to Install Updates" > text: "There are updates available, but they can't be installed because > software installation for Minefield has been disabled. Please contact your > system administrator for assistance." > - the "don't check" button shouldn't be here > - the "Finish" button should be at the right, selected by default s/Minefield/%S/ > screen 7: > panel title: "Installing Compatibility Updates" > text: "Downloading and installing updates to your add-ons" > progress indication text is good I'd go with "&#133;" ("...") after "add-ons" (just to coincide with progress bar). > screen 8: > panel title: "Compatibility Updates Installed" > text: "%S has installed the updates to your add-ons." Shouldn't you mention something about restarting here? > screen 9: > panel title: "No Compatibility Updates Found" > text: "%S was unable to find any updates for your add-ons to make them > compatible with this version. These updates may be available in the future. %S > will check periodically and inform you when compatible versions of your add-ons > are found." Do you really want to say "versions" here? Other references to "version(s)" in earlier proposed text are directed towards the application itself and not the add-on. How about "updates" (or a synonym) instead of "versions"? > screen 10: > panel title: "Some Compatibility Updates Found" > text: "%S was unable to find any updates for some of your add-ons to make > them compatible with this version. These updates may be available in the > future. %S can check periodically and inform you when compatible versions of > your add-ons are found." Title and text contradict each other. You say "Updates Found" in title but "unable to find" in text. Also, you might need to explain that some updates were found and that a restart is required to finish installation. Don't spend too much time on one side of the fence. I'd straddle the fence by spending a sentence or two on both the found and not found updates. See comment in earlier screen 9 outline. > screen 11: > panel title: "No Compatibility Updates Found" > text: "%S was unable to find any updates for your add-ons to make them > compatible with this version. These updates may be available in the future. %S > can check periodically and inform you when compatible versions of your add-ons > are found." See comment in earlier screen 9 outline.

(In reply to comment #15) > beltzner, from comment #7 there are a couple of more. > installErrorDescription=The following components could not be installed due to > errors (the file could not be downloaded, was corrupt, or for some other > reason). Need something after and/or before "or for some other reason" to end the sentence gracefully. Maybe "or for some other reason was invalid" or "or was for some other reason invalid". Also, I bet s/components/add-ons/. > checkingErrorDescription=%S could not check for updates to the following > components (either the update server(s) did not respond, or the update > service(s) were not found). Maybe "for the following" instead of "to the following" in the first sentence (however, the for...for sounds weird). Again, probably s/components/add-ons/. The comma after "respond" needs to be removed.

(In reply to comment #16) > > screen 3: > > panel title: "Checking for Compatibility Updates" > > text: "Checking for available updates to incompatible add-ons ..." > > - just need the text below the progress indicator, remove the stuff above > > Remove the space between "add-ons" and "...". Also, shouldn't you be using > "&#133;" instead of "..." here? I didn't get the memo... why are any decimals in dtd's being changed to the html entity equivalent? > > screen 4: > > panel title: "Unable to Install Updates" > > text: "There are updates available, but they can't be installed because > > software installation for Minefield has been disabled. Please contact your > > system administrator for assistance." > > - the "don't check" button shouldn't be here > > - the "Finish" button should be at the right, selected by default > > s/Minefield/%S/ Having the value is better for me so there is no question whether it should be the short brand name, full brand name, app version, etc. > > screen 8: > > panel title: "Compatibility Updates Installed" > > text: "%S has installed the updates to your add-ons." > > Shouldn't you mention something about restarting here? See previous comment #5 about restart not being necessary.

(In reply to comment #18) > (In reply to comment #16) > > > screen 3: > > > panel title: "Checking for Compatibility Updates" > > > text: "Checking for available updates to incompatible add-ons ..." > > > - just need the text below the progress indicator, remove the stuff above > > > > Remove the space between "add-ons" and "...". Also, shouldn't you be using > > "&#133;" instead of "..." here? > I didn't get the memo... why are any decimals in dtd's being changed to the > html entity equivalent? beltzner used &#133; in screen 1, I believe, so either way, the same format needs to be used in all of them. > > > screen 4: > > > panel title: "Unable to Install Updates" > > > text: "There are updates available, but they can't be installed because > > > software installation for Minefield has been disabled. Please contact your > > > system administrator for assistance." > > > - the "don't check" button shouldn't be here > > > - the "Finish" button should be at the right, selected by default > > > > s/Minefield/%S/ > Having the value is better for me so there is no question whether it should be > the short brand name, full brand name, app version, etc. Meaning you prefer the %S here, correct? > > > screen 8: > > > panel title: "Compatibility Updates Installed" > > > text: "%S has installed the updates to your add-ons." > > > > Shouldn't you mention something about restarting here? > See previous comment #5 about restart not being necessary. Oops, sorry, didn't see that. :)

(In reply to comment #19) > Meaning you prefer the %S here, correct? No, %S may mean short brand name, full brand name, or some other string so providing either the actual text or a generic version of the actual text (e.g. shortBrandName, fullBrandName, etc.) removes any question as to what is meant.

repost: beltzner, from comment #7 there are a couple of more. installErrorDescription=The following components could not be installed due to errors (the file could not be downloaded, was corrupt, or for some other reason). checkingErrorDescription=%S could not check for updates to the following components (either the update server(s) did not respond, or the update service(s) were not found). See attachment #228230 [details] [edit] for context

Comment on attachment 228222 [details] [diff] [review] patch - forgot a file in the last one r=sspitzer, robert explained this patch to me face-to-face.

(In reply to comment #21) > beltzner, from comment #7 there are a couple of more. Gah, I'm well beyond my initial ETA of 20 minutes :( Your strings, sire: > installErrorDescription=The following components could not be installed due to > errors (the file could not be downloaded, was corrupt, or for some other > reason). "%S was unable to install the updates for the following add-ons:" > checkingErrorDescription=%S could not check for updates to the following > components (either the update server(s) did not respond, or the update > service(s) were not found). "%S was unable to successfully check for updates for the following add-ons:" (These strings assume, as your screenshot shows, that the error reasons will be shown in the textbox with the add-on names. My feeling is that there's no need to say that it was due to an error - the very fact that we were unable to perform the action is, in itself, an error!) Also, the "OK" button should become part of the wizard, or become a "Next" button or something to fit into the overall flow. I also folded in your and Reed's comments to get: screen 1: panel title: "Checking Add-ons Compatibility" progress text: "Checking your add-ons&#133;" screen 2: panel title: "Found Incompatible Add-ons" text: "The following add-ons are not compatible with this version of %S and have been disabled:" text: "%S can check to see if compatible updates to these add-ons are available." screen 3: panel title: "Checking for Compatibility Updates" text: "Checking for available updates to incompatible add-ons&#133;" progress text: "Checking for update to %S." screen 4: panel title: "Unable to Install Updates" text: "There are updates available, but they can't be installed because software installation for %S has been disabled. Please contact your system administrator for assistance." - the "don't check" button should be a disabled "Cancel" button screen 5: panel title: "Found Compatibility Updates" text: "Select the updates you would like to install:" screen 6: panel title: "Found Compatibility Updates" text: "Select the updates you would like to install:" text: "The selected updates can't be installed because software installation is currently disabled. You can change this setting below." screen 7: panel title: "Installing Compatibility Updates" text: "Downloading and installing updates to your add-ons&#133;" progress text as current screen 8: panel title: "Compatibility Updates Installed" text: "%S has installed the updates to your add-ons." screen 9: panel title: "No Compatibility Updates Found" text: "%S was unable to find any updates for your incompatible add-ons. %S will continue to check periodically and inform you when compatible updates for these add-ons are found." screen 10: panel title: "Some Compatibility Updates Found" text: "%S was unable to find any updates for some of your incompatible add-ons. %S will continue to check periodically and inform you when compatible updates for these add-ons are found." - checkText is good as is screen 11: panel title: "No Compatibility Updates Found" text: "%S was unable to find any updates for your incompatible add-ons. %S will continue to check periodically and inform you when compatible updates for these add-ons are found." - checkText is good as is

(In reply to comment #23) > screen 10: > panel title: "Some Compatibility Updates Found" Er, panel title: "Some Compatibility Updates Not Found"

I opened Bug 345353 for the string changes that aren't required to fix the broken code that is fixed by this bug since it will require additional code. Re-opening bug... I haven't landed this yet. :P

(In reply to comment #23) > > installErrorDescription=The following components could not be installed due to > > errors (the file could not be downloaded, was corrupt, or for some other > > reason). > > "%S was unable to install the updates for the following add-ons:" We provide decent errors here so this is good. > > checkingErrorDescription=%S could not check for updates to the following > > components (either the update server(s) did not respond, or the update > > service(s) were not found). > > "%S was unable to successfully check for updates for the following add-ons:" We don't provide good errors here so I'm going to use: %S was unable to successfully check for updates for the following add-ons (either the update server(s) did not respond, or the update service(s) were not found): > Also, the "OK" button should become part of the wizard, or become a "Next" > button or something to fit into the overall flow. This is an information only dialog that can be opened from the wizard and standard is to use an OK button in this case since it is not part of the wizard pages.

Checked in to trunk

Comment on attachment 228222 [details] [diff] [review] patch - forgot a file in the last one This code needed some major cleanup since the last time it was changed.

Comment on attachment 228222 [details] [diff] [review] patch - forgot a file in the last one a=mconnor on behalf of drivers for checkin to the 1.8 branch

Checked in to MOZILLA_1_8_BRANCH