87177 - Textfield on "Offline and Disk Space" is editable regardless of checkbox status.

Status: VERIFIED FIXED

(SeaMonkey :: Preferences, defect, P3)

Description

[André Dahlqvist]

[Build 2001-06-20-17 on Linux]

The textfield on the "Offline and Disk Space" panel in Preferences should only
be editable if the checkbox beneeth it is checked. Currently it is always enabled.

I guess the fix for this one is similar to that of bug 86899.

Comment 1

[sairuh (rarely reading bugmail)]

more like bug 86986...

Comment 2

[sairuh (rarely reading bugmail)]

handing over to diane [guessing by the offline gui bugs she fixed]. punt as needed.

Comment 3

[Diane Sun]

Attachment: Fix in js to disable text field if checkbox not checked

Comment 4

[Diane Sun]

Attachment: screen shot

Comment 5

[Diane Sun]

There is an issue: check/uncheck the checkbox, the text field availability is 
not affected on current display.  Question:  how does  Account Manager's panel, 
preference gets updated  real-time whenever a preference checkbox/groupradio is 
checked.

Comment 6

[Diane Sun]

Attachment: New complete fix with broadcast update

Comment 7

[Diane Sun]

Request R & SR,   Bhuvan and Seth?

Comment 8

[racham]

Diane,

I have some minor comments.

* What's the patch behind the screenshot (id:39724) you posted ?

* In your latest patch, things look good. I have one comment though. I noticed
that variable name 'bc' being used in couple of places. I can expand that to
broadcaster as I know the code. So, please use longer and more context sensitive
names (broadcasterDownloadLimitSize) so that it will be easy to understand the code.

* Do post another screenshot with your latest fix.

* Run some tests.

&& then r=bhuvan.

Comment 9

[Diane Sun]

Attachment: Screenshot

Comment 10

[Diane Sun]

Attachment: Old fix, new version, changed names, co-ordinate with XUL file changes.

Comment 11

[Diane Sun]

Request  SR for attachment 43607 [details] [diff] [review],  Seth?

Comment 12

[racham]

I was talking about the var 'bc' in the following statements. May be you changed
and probably forgot to post the patch with var name changed to a more meaningful
one.

+    var  bc = document.getElementById("bc_notDownload");
+    var  bc = document.getElementById("bc_downloadMsg");
+    var  bc = document.getElementById("bc_keepMsg");
+    var  bc = document.getElementById("bc_removeBody");

Also, when you are there, can you add comments on the top of each of these
functions.

thanks
bhuvan

Comment 13

[Diane Sun]

Attachment: name changes

Comment 14

[Diane Sun]

Request  SR for attachment 43627 [details] [diff] [review], not 43607 now.

Comment 15

[(not reading, please use [e-mail] instead)]

sorry for the delay.  feel free to send me mail directly when you are ready for
a super review.

you've got some duplicate code that you can remove.

onCheckKeepMsg() is fine, but these are duplicated code:

onCheckNotDownload()
onCheckDownloadMsg()
onCheckRemoveBody()

example, instead of doing:

+ onCheckNotDownload();

do

+ onCheckItem("bc_notDownload","offline.notDownload");

onCheckItem would be something like this:

function onCheckItem(broadcasterElementId, checkElementId)
{
    var broadcaster = document.getElementById(broadcasterElementId);
    var checked = document.getElementById(checkElementId).checked;

    if(checked) {
        broadcaster.removeAttribute("disabled");
    }
    else {
        broadcaster.setAttribute("disabled", "true");
    }
}

Comment 16

[Diane Sun]

Like to reduce the duplicate code, BUT IT WILL NOT WORK.
Remember in the XUL file, there are
oncommand=onCheckNotDownload()
oncommand=onCheckDownloadMsg()
oncommand=onCheckRemoveBody()
oncommand=onCheckKeepMsg()
to make each click work real-time.

Please SR again.

Comment 17

[(not reading, please use [e-mail] instead)]

sure it will work.

in your xul,

you change

oncommand="onCheckNotDownload();"
to
oncommand="onCheckItem('bc_notDownload','offline.notDownload');"

Comment 18

[Diane Sun]

Yes, I know that. But it makes xul file complicated.  
If you prefer that way, I can make the change.

Comment 19

[(not reading, please use [e-mail] instead)]

duplicated code is bloat and is harder to maintain.

I don't think it makes it more complicated.

yes, please go with the suggested approach.

Comment 20

[Diane Sun]

Attachment: Remove dupliate code in js

Comment 21

[(not reading, please use [e-mail] instead)]

sr=sspitzer on the last attachment.

Comment 22

[Blake Ross]

Missed 0.9.3.

Comment 23

[Diane Sun]

checked in.

Comment 24

[sairuh (rarely reading bugmail)]

hrm, this is still an issue: in the Disk Space section, the checkbox for
"Compact folders when it will save over [field] KB" is still editable when it is
not selected.

reopening --however, let me know if that's covered by another bug!

tested using:

linux, 2001.08.15.14-comm
winnt, 2001.08.15.06-comm
mac OS 9.1 [emul on X], 2001.08.15.08-comm

will attach screenshot...

Comment 25

[sairuh (rarely reading bugmail)]

Attachment: unselected checkbox and editable textfield --note cursor

Comment 26

[racham]

Though this bug is opened keeping AccountManager panel in the view, we should
also fix the global Offline & Disk space panel problem as pointed out by Sairuh.
It's anyway the same procedure.

Comment 27

[Diane Sun]

Attachment: screeshot

Comment 28

[Diane Sun]

Attachment: Fix for pref

Comment 29

[Diane Sun]

Request R & SR for additional fix for preference. attachment 46251 [details] [diff] [review].
Buhvan, Seth?

Comment 30

[(not reading, please use [e-mail] instead)]

looks reasonable.

sr=sspitzer

Comment 31

[racham]

r=bhuvan.

Please make sure that you change one of variable names from 'el' to something
more context sensitive (& test once) before you check in. thanks.

Comment 32

[Diane Sun]

check in.

Comment 33

[Stephen Walker]

reopening, the patch is completely wrong. the textfield is disabled when i 
check the checkbox & the textfield is enabled when the checkbox isn't checked.

Comment 34

[(not reading, please use [e-mail] instead)]

Attachment: patch, reuse existing code and fix the problem walk84 found.

Comment 35

[Stephen Walker]

r=walk84

Comment 36

[(not reading, please use [e-mail] instead)]

fixed.

Comment 37

[sairuh (rarely reading bugmail)]

vrfy fixed.

linux, 2001.08.22.08-comm
winnt, 2001.08.22.09-comm
mac 9.1, 2001.08.22.08-comm