Invalid history pref entry is reset to 0, not previous value

RESOLVED FIXED in mozilla1.0.1



History: Global
17 years ago
16 years ago


(Reporter: Claudius Gayle, Assigned: Blake Ross)



Firefox Tracking Flags

(Not tracked)



(1 attachment)



17 years ago
this little nugget sprung from bug 81415.

In the Navigator-->History pref panel invalid entries into the session history
textfield are reset to '0' when the prefs window is closed.

to reproduce:
1. Choose Edit-->Preferences.
2. Select the Navigator-->History panel.
3. In either textfield, enter and invalid value e.g. 'p'.
4. Click 'OK'
5. Reopen prefs to the same panel.
Actual results:
The text entry is reset to '0' which is neither the previous entry OR the
default setting.
Expected results:
invalid entry should be reset to previous value to avoid the
destructive/unexpected possibilities of expirig history after 0 days. If
remembering the previous value turns out to be challenging, default values could
be a stop-gap measure.
possible backend issue?

Comment 2

17 years ago
No, this is a UI issue. There was a similar bug to this filed about setting the
cache size to a value and that value being converted to a different number. The
issue has something to do with automatic value conversion in XUL/JS as I recall.

Adding eddyk as I think he did the investigation/work on the cache value bug.

Comment 3

17 years ago
go read bug 81415, that's why i referenced it and bugzilla provides a nice
hotlink. One could probably isolate blake's exact checkin related to this bug.


17 years ago
Priority: -- → P3
Target Milestone: --- → mozilla0.9.6

Comment 4

17 years ago
I haven't looked into this issue very closely, but it does seem to be similar to
the bug 63117.

I wonder what the status of bug 66163 is?  That would take care of this and
other similar issues.

Other possible solutions would be to make nsPrefWindow/nsWidgetStateManager
handle these cases more robustly, or to have validation js code in every pref panel.

Comment 5

17 years ago
Eddy, yes bug 63117 is the one I was thinking of.

Claudius, I have read bug 81415, which references bug 66163 as being the right fix.

Blake's change (in pref-history.xul) forces the text input field to be 0 if it
is invalid:
  if (maxSize < 0) {
    // set the pref to 0 so the UI reflects the proper value
    if (panelIsShown)
      document.getElementById("shistMax").value = "0";
      data['shistMax'].value = "0";
    maxSize = 0;

If the backend prefs code is told to set the value to 0, it will. On the other
hand, if the backend prefs code is not called to change the value, the value
will remain unchanged. Either way, the backend is only doing what the frontend
is telling it to do.

The value of "browser.sessionhistory.max_entries" in prefs.js will reflect the
last value given to the pref backend.

Comment 6

17 years ago
Moving to mozilla0.9.7.
Target Milestone: mozilla0.9.6 → mozilla0.9.7

Comment 7

17 years ago
-> mozilla1.0.1
Target Milestone: mozilla0.9.7 → mozilla1.0.1

Comment 8

17 years ago
I'm still a little skittish about setting people's history pref to '0'. Is there
not some simple change to be made to mediate the behavior in the meantime. If we
can't completely fix it before 1.0.1?

Comment 9

17 years ago
-> History for stop-gap measure.
Assignee: sgehani → blakeross
Component: Preferences → History: Global
QA Contact: sairuh → claudius

Comment 10

16 years ago
Bug 66163 is the real fix for this.  I agree, we shouldn't default to 0. I'm
going just take the easy way out and set to 50 (the default).

Comment 11

16 years ago
Created attachment 69324 [details] [diff] [review]


Comment 12

16 years ago
Fix checked in. The patch also fixes entering non-alphanumeric characters, so
that should be tested when verifying.

Note that currently entering 0 into *any* textbox in preferences, pressing OK,
and then reopening prefs shows no value in that textbox, instead of 0 (even
though the pref was correctly set). So I didn't introduce that bug, but I'm
going to file it and try to fix it.
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 13

16 years ago
*** Bug 120311 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.