Last Comment Bug 376975 - use "number" type for textboxes in options dialog
: use "number" type for textboxes in options dialog
Status: RESOLVED FIXED
: polish
Product: Thunderbird
Classification: Client Software
Component: Preferences (show other bugs)
: Trunk
: All All
: P2 normal (vote)
: Thunderbird 3.0b1
Assigned To: Phil Ringnalda (:philor)
:
Mentors:
Depends on: 350257 350334 377114 443846
Blocks: 226551
  Show dependency treegraph
 
Reported: 2007-04-09 20:41 PDT by (not reading, please use seth@sspitzer.org instead)
Modified: 2008-10-20 15:36 PDT (History)
7 users (show)
mkmelin+mozilla: wanted‑thunderbird3+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Mac screenshot (12.53 KB, image/png)
2007-04-09 22:53 PDT, Phil Ringnalda (:philor)
no flags Details
Fix v.1 (6.05 KB, patch)
2008-04-28 00:05 PDT, Phil Ringnalda (:philor)
no flags Details | Diff | Splinter Review
Unrotted (6.63 KB, patch)
2008-07-06 22:33 PDT, Phil Ringnalda (:philor)
mkmelin+mozilla: review-
Details | Diff | Splinter Review
Fix v.2 (3.66 KB, patch)
2008-10-19 15:33 PDT, Phil Ringnalda (:philor)
mkmelin+mozilla: review+
Details | Diff | Splinter Review

Description (not reading, please use seth@sspitzer.org instead) 2007-04-09 20:41:36 PDT
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...
Comment 1 Scott MacGregor 2007-04-09 21:50:25 PDT
thanks seth!

Phil or Magnus, feel like taking a crack at this? 
Comment 2 Phil Ringnalda (:philor) 2007-04-09 22:53:17 PDT
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 3 Phil Ringnalda (:philor) 2007-04-09 23:31:16 PDT
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.
Comment 4 Phil Ringnalda (:philor) 2007-04-10 14:45:29 PDT
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.
Comment 5 Phil Ringnalda (:philor) 2007-04-21 18:08:49 PDT
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?
Comment 6 Phil Ringnalda (:philor) 2008-04-28 00:05:40 PDT
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.)
Comment 7 Phil Ringnalda (:philor) 2008-07-06 22:33:43 PDT
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 8 Magnus Melin 2008-07-07 09:27:03 PDT
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.
Comment 9 Magnus Melin 2008-07-07 09:31:01 PDT
For timeout and compact, maybe use increment="10" (and 100)?
Comment 10 Magnus Melin 2008-08-23 05:39:27 PDT
Comment on attachment 328293 [details] [diff] [review]
Unrotted

Minusing based on previous comments.
Comment 11 Phil Ringnalda (:philor) 2008-10-19 15:33:50 PDT
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?
Comment 12 Magnus Melin 2008-10-20 11:32:42 PDT
Just noting down that the magic number for purge_threshold (2097151) is 2 GiB, (2^20)*2.
Comment 13 Magnus Melin 2008-10-20 11:38:21 PDT
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)
Comment 14 Phil Ringnalda (:philor) 2008-10-20 15:36:42 PDT
http://hg.mozilla.org/comm-central/rev/131887be33a5

Note You need to log in before you can comment on or make changes to this bug.