In the new prefwindow, the label of the "Clear Now" button in the Privacy pane shouldn't contain ellipsis when set to not ask before clearing. So it should be "Clear Now..." if clicking the button would show the dialog, otherwise just "Clear Now". This is a potential dataloss issue because a user might expect the dialog to come up because of the ellipsis.

Updates the "Clear Now..." button label when the privacy pane is initialized, and after opening the sanitize settings dialog.

Oh, I forgot to change the updateClearNowButtonLabel() comment. It should be: + * Sets the label of the "Clear Now..." button according to the + * privacy.sanitize.promptOnSanitize pref. I've fixed that locally.

The menu item in the History menu is already updated dynamically via SanitizeListener() in browser.js.

I take it that this button always clears the private data regardless of the "Always ask me" setting in the Clear Private Data settings? That actually seems unwise to me, but if that's the case, then yes, the ellipses should vanish.

(In reply to comment #4) > I take it that this button always clears the private data regardless of the > "Always ask me" setting in the Clear Private Data settings? That actually seems > unwise to me, but if that's the case, then yes, the ellipses should vanish. Actually, it's sorta broken that way -- it only shows a dialog if "Always ask me" is checked. Note that this also applies to Tools>Clear Private Data...

Comment on attachment 230282 [details] [diff] [review] patch r=me, but use setAttribute instead of .label =

Please do use .label, the button binding has this property (from basetext).

(In reply to comment #4) > I take it that this button always clears the private data regardless of the > "Always ask me" setting in the Clear Private Data settings? That actually seems > unwise to me, but if that's the case, then yes, the ellipses should vanish. If the "ask me before clearing" checkbox in Privacy->Private Data->Settings is checked, clicking the "Clear Now..." button displays a dialog with "Cancel" and "Clear" buttons. If it is not checked however, clicking the Clear button just clears without asking. In that case the button label should be "Clear Now", without ellipsis. That's what this bug is about.

Checked in on trunk.

Comment on attachment 230282 [details] [diff] [review] patch Branch checkin depends on the branch landing of the new prefwindow, but I could already add the new string.

Forgot to mention that I kept using .label= as Mano requested. And yes, I've tested it, and it works just fine.

Comment on attachment 230282 [details] [diff] [review] patch a=mconnor on behalf of drivers for checkin after the new prefwindow lands

To save localizers the trouble of having slightly-out-of-sync DTDs (and thus to allow a direct copy over of trunk DTDs to branch code without changes, hopefully ameliorating the problem of dealing with wicked huge l10n changes on branch), I rolled this patch into the commit of the prefwindow to branch. I did a final build of everything and did a quick functionality smoketest for all prefwindow features, including this one, to make sure everything worked correctly. Consequently, this is fixed on branch.

Comment on attachment 230282 [details] [diff] [review] patch >Index: mozilla/browser/components/preferences/privacy.js >+ updateClearNowButtonLabel: function() >+ { >+ var prefBranch = Components.classes["@mozilla.org/preferences-service;1"] >+ .getService(Components.interfaces.nsIPrefBranch); >+ var clearNowButton = document.getElementById("clearDataNow"); >+ if (prefBranch.getBoolPref("privacy.sanitize.promptOnSanitize")) >+ clearNowButton.label = clearNowButton.getAttribute("label1"); // "Clear Now..." >+ else >+ clearNowButton.label = clearNowButton.getAttribute("label2"); // "Clear Now" >+ }, >+ >+ /** > * Displays the Clear Private Data settings dialog. > */ > showClearPrivateDataSettings: function () > { > document.documentElement.openSubDialog("chrome://browser/content/preferences/sanitize.xul", > "", null); >+ this.updateClearNowButtonLabel(); > }, Hmm, the button label should be updated after leaving the Clear Private Data settings dialog. But on Windows, where browser.preferences.instantApply is false by default, the pref and therefore the button label is only updated after clicking OK on the main prefwindow... How can I check whether the promptOnSanitize pref *will be* changed (and to which value) when the user clicks OK?

(Note to self: get the <preference> with the name "privacy.sanitize.promptOnSanitize", which is inserted into the parent prefwindow after leaving the child dialog, and look at preference.value.)

If you plan on doing a second patch, a couple requests: - change the new method name to start with a "_" -- convention in prefwindow files is to "_"-prefix any method only used within the containing JS file (and not within XUL) and to use an unprefixed name for anything called from outside the containing JS file - except in very rare circumstances (e.g. populating the list of feed readers), it should never be necessary to manually get a preference value -- create a preferences element instead, and the .value property on it will always be what's shown in UI (I believe even with subdialogs as happens here), on systems with or without instant-apply - random style nits in comments (fixed in the branch checkin, but trunk's still screwy): > /** >- * Sets up the UI for the number of days of history to keep. >+ * Sets up the UI for the number of days of history to keep, and updates the >+ label of the "Clear Now..." button. > */ Missing * >- /** >+ /** >+ * Removes the ellipsis from the "Clear Now..." button if >+ * privacy.sanitize.promptOnSanitize is false. >+ */ >+ /** > * Displays the Clear Private Data settings dialog. Funky "/**" indentation >+ updateClearNowButtonLabel: function() Space between "function" and "()"

Adds a <preference> for the promptOnSanitize pref and checks that. The only drawback is that the Error Console shows this message 7 times after clicking OK in the Clear Private Data settings dialog: <preference> with id='' and name='' has unknown type ''.

Comment on attachment 231506 [details] [diff] [review] patch to fix on Windows This is now part of bug 345769.