for tbird 3, consider using neil's new "number" type for the textboxes in options dialog and account manager example: <textbox type="number" min="1" max="20"/> see http://developer.mozilla.org/en/docs/XUL:textbox examples from options dialog: autosave timeout 5 mins wrap at 72 chars wait 5 seconds before marking as read connection timeout compact disk space etc... examples from account settings: port disk space etc...
10 years ago
10 years ago
thanks seth! Phil or Magnus, feel like taking a crack at this?
Created attachment 261102 [details] Mac screenshot While I understand the "use it somewhere so the bugs are worth fixing so it can be used somewhere" cycle, I'd just as soon not implement it on Mac right yet.
Comment on attachment 261102 [details] Mac screenshot Blah, I'm too used to Cocoa widgets being broken; that's actually just Tb's preferences.css setting an appearance for any button, even a spinbutton.
Though Cocoa widgets are somewhat at fault, since bug 371080 "fixed" things for Firefox by removing that rule from Fx, but not Tb or Sb.
Department of UI Design: in toolkit prefwindows, there's an input event handler that syncs the value of the input with the value of the preference element every time you press a key, which means that every keypress accesses the numberbox's value, which means that the numberbox calls _validateValue and changes to the value of the max attribute as soon as you type something that makes it overflow, and then stays there. In the account manager, nothing watches every keypress, so if you type 99999 in an input with max="65535", it doesn't change to 65535 until you focus another element, or, more likely, close the pane or window, when it's too late to see that you can't have what you entered. So, do we want to live with the difference, convince SeaMonkey they'd like to have an input handler firing all the time, find some way to only do it for Tb, or not have numberboxes in the account manager?
Created attachment 318100 [details] [diff] [review] Fix v.1 Mmm, magic numbers! I think I got them right: * mail.compose.autosaveinterval is minutes used as milliseconds in a setTimeout, so PR_INT32_MAX / 60000 * mailnews.wraplength is... confusing, other than being used in a PR_INT32 * mailnews.mark_message_read.delay.interval is seconds used as milliseconds in a setTimeout, so PR_INT32_MAX / 1000 * mailnews.tcptimeout is used as a PR_UINT32, but for unexplained reasons the TIMEOUT_READ_WRITE is set to the pref value, TIMEOUT_CONNECT is set to the pref value + 60, so PR_UINT32_MAX - 60 (My question about what we want to do about the account manager was answered quite a while ago, by someone just landing a patch to give it numberboxes, even though they don't work quite as prettily.)
Created attachment 328293 [details] [diff] [review] Unrotted I have the feeling there was some reason why I tagged dmose for this, but I can't imagine what it was, unless it was retribution for something (in which case, since in the meantime I reviewed completely rotting my own patch, now I owe him twice over).
Comment on attachment 328293 [details] [diff] [review] Unrotted On one side, min="0" is the default and not needed. But then again, the min shouldn't be 0 for any of these now, should it? markAsReadDelay: min 1 sec connectionTimeoutBox: min 1 sec? offlineCompactFolderMin: don't know, but anything under 10 KB is ridiculous I also wonder a bit about hidespinbuttons="true". Do we have some policy on that? E.g. the connection dialog ports don't use it (as firefox didn't). I think it might be best to use the spin buttons for consistency, though I'm certainly open for other opinions.
For timeout and compact, maybe use increment="10" (and 100)?
Comment on attachment 328293 [details] [diff] [review] Unrotted Minusing based on previous comments.
Created attachment 343849 [details] [diff] [review] Fix v.2 The longer I wait, the easier it gets, as we remove more and more of them :) Dropped my confused use of min="0", but setting a non-zero min will need a separate bug, where someone does some cute initialization to set the checkboxes on init, since if someone set the numeric pref to 0 manually, we don't want to change their preference and turn on autosave/compact just because they opened the prefwindow. There was going to be a policy (at least, an Fx policy) on not showing spinbuttons for things where you wouldn't want to go one-by-one, but as you say, Fx's spinbuttons on ports make it clear that didn't happen, and it also wasn't actually what I wanted, increment was. However, as dveditz says every time we start talking about how cheap disk space is, that's also "how much mail that I thought I deleted will you keep around even though I don't know it's still there?" so we don't want to just base it on the size of our disks. How about 10?
Just noting down that the magic number for purge_threshold (2097151) is 2 GiB, (2^20)*2.
Comment on attachment 343849 [details] [diff] [review] Fix v.2 r=mkmelin, though I still think "upgrading" users that misuse those fields to be 0 = disabled would be a good thing. For purge_threshold it doesn't look like setting it to 0 would disable it either. (see nsMsgDBFolder::AutoCompact GetPurgeThreshold)