LDAP panel content doesn't fit

VERIFIED FIXED in mozilla0.9.2

Status

P1
normal
VERIFIED FIXED
18 years ago
14 years ago

People

(Reporter: marina, Assigned: srilatha)

Tracking

(Blocks: 2 bugs, {l12y})

Trunk
mozilla0.9.2
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: Have a fix, Requested review)

Attachments

(9 attachments)

(Reporter)

Description

18 years ago
**** observed with 2001-05-11-04 build **** on Win2K
in today's build Prefs window is not resizable therefor some Preferences can not
be used (eg. can not try LDAP for Address Book, all i see is Autocomplete
against Local AB)

Comment 1

18 years ago
See bug 79948.
(Reporter)

Comment 2

18 years ago
bug #79948 suggest to make a Prefs dlgbox non-resizable , this makes some Prefs
to be unusable ( for ex : i can not use LDAP Pref for ABook, they don't fit the
size of the actual window)

Comment 3

18 years ago
SSL prefs do not fit anymore in the prefs windows too.

Comment 4

18 years ago
non-resizeable on linux too -> all
OS: Windows 2000 → All
Hardware: PC → All

Comment 5

18 years ago
*** Bug 80308 has been marked as a duplicate of this bug. ***

Comment 6

18 years ago
This is due to fix for bug 79948, over to ben
Assignee: mcafee → ben
Depends on: 79948

Comment 7

18 years ago
79948 was reopened, let's just handle it there.

*** This bug has been marked as a duplicate of 79948 ***
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → DUPLICATE
okidoke
Status: RESOLVED → VERIFIED
No longer depends on: 79948
undupping, per comments in bug 79948.

pls reassign to the appropriate engr who works on the LDAP pref content. thx!
Status: VERIFIED → REOPENED
Resolution: DUPLICATE → ---
Summary: Preferences window is not resizable → LDAP panel content doesn't fit
change product
Component: Preferences → Address Book
Product: Browser → MailNews
over to default owner. 
Assignee: ben → chuang
Status: REOPENED → NEW
QA Contact: sairuh → esther
Keywords: nsbeta1, ui
*** Bug 80314 has been marked as a duplicate of this bug. ***

Comment 13

18 years ago
Copying Michele and adding l12y keyword, while fixing this we also need to 
consider that l10n builds will need even more space.
Keywords: l12y

Comment 14

18 years ago
reassigning to srilatha.
Assignee: chuang → srilatha
(Assignee)

Comment 15

18 years ago
Jennifer, any ideas?
Now, we cannot see the ldap settings at all because the pref window is not 
resizeable
Priority: -- → P1
Target Milestone: --- → mozilla0.9.1

Comment 16

18 years ago
Created attachment 34440 [details]
Updated Pref Panel

Comment 17

18 years ago
My attempt at less wordy version above. 
(Assignee)

Comment 18

18 years ago
Do we have enough space to add another checkbox

X Do not search directory if there is a match in local Address Book
or may be you can come up with better wording :-)
(Assignee)

Updated

18 years ago
Blocks: 17880

Comment 19

18 years ago
Perhaps 'Addressing' instead of 'Address Books'?

Email addresses from -your- messages /are/'can be' added to a local address 
book, "Collected Addresses".  Add email addresses from:

...

Directory<-tail of y chopped off :-(

Old fassioned style said to put closing quote outside of punctuation, I'm 
guessing that style was eventually killed for cause?

Comment 20

18 years ago
There might be room.

If so, would suggest similar to 4.x:

If there is a match in your local address books:
[ ] Do not search in the directory

Comment 21

18 years ago
Created attachment 34452 [details]
Updated screenshot
(Assignee)

Comment 22

18 years ago
Jennifer looks like 
If there is a match in your local address books:
[ ] Do not search in the directory
will fit in the panel. I tried it in my debug build. 
One more thing,
checkbox for Limit the collected address book ......
is missing in the screen shot.

Comment 23

18 years ago
erm, -your- == remove.  Messages in newsgroups aren't the user's.

Comment 24

18 years ago
Created attachment 34542 [details]
OK, this should be correct now

Comment 25

18 years ago
timeless, sorry missed the "remove 'your' " the first time around. :-)  As for 
"Addressing" vs "Address Book" not sure one is better than the other. "Collected 
AB" isn't really "Addressing" and "Autocomplete" isn't really "Address Book". 
"Addressing" might be better, up to srilatha if she wants to make that change 
this close to ns beta (or maybe we can do this change after ns beta?).

srilatha, I added the checkbox in front of "Limit the collected address book..." 
but please be sure that the checkbox is NOT aligned with the "Add email 
addresses from:" options (like it is in current builds). Its not part of that 
group. Also, if its a checkbox, the "." should be removed after "cards".
(Assignee)

Comment 26

18 years ago
I think we should leave it as "Address Books". 
Jennifer, I will make sure that "Limit the collected Address ...." will not be 
part of the group.
QA Contact: esther → yulian
(Assignee)

Comment 27

18 years ago
Created attachment 34576 [details] [diff] [review]
Modified based on the spec
(Assignee)

Comment 28

18 years ago
Hoping this is going to be the final revision of the spec.
ccing mohan for a review
Seth can you sr the patch.
Status: NEW → ASSIGNED
FWIW, I think Addressing is a better characterization of the name of this pane,
because the directory server stuff is not actually related to address books.

Comment 30

18 years ago
I concur with Dan, please label this pane, "Addressing" not "Address Books".

(Assignee)

Comment 31

18 years ago
Created attachment 34617 [details] [diff] [review]
Patch version2. changed to "Addressing".

Comment 32

18 years ago
r=mohanb; the content does not fit on Mac; 
(Assignee)

Comment 33

18 years ago
On Mac we can see all the content except the border for Address Autocompletion 
box.

Comment 34

18 years ago
Setting target milestone to 0.9.2 (check it in anytime, even before, when the
tree is open for). Per PDT triage.
Target Milestone: mozilla0.9.1 → mozilla0.9.2
(Assignee)

Updated

18 years ago
Whiteboard: Have a fix, Requested review
what happened to the js for the enabling?  did that get moved somewhere else?
(Assignee)

Comment 36

18 years ago
No, it was not moved. Since we do not have the check box for 
Enable Email Address Collection, we do not need enabling.
Before this change, the enabling/disabling of the remaining checkboxes was done 
by the value of this checkbox.
right, I should have figured that out from the patch.

sr=sspitzer
(Assignee)

Comment 38

18 years ago
I am glad you asked me that question. It made me search for references to the 
pref for the checkbox "Enable Email Address Collection" which is 
"mail.collect_email_address"
and I did find tehm in nsAbAddresscollector.
I have a patch which removes the references to the pref 
"mail.collect_email_address".
(Assignee)

Comment 39

18 years ago
Created attachment 34700 [details] [diff] [review]
patch part 2 which removes references to pref "mail.collect_email_address"
I take back my sr.  I'm totally confused.

first, why are you removing that pref from the UI? 
second, why are you removing that pref from the back end?

don't we still want to allow people disable automatic collection?
again, don't check any of this in yet.

adding bienvenu and putterman so they can comment about the
"mail.collect_email_address" pref

Comment 42

18 years ago
yes, I'm confused too - why would we remove that pref? Has it been replaced by
another pref?
(Assignee)

Comment 43

18 years ago
The user can still disable automatic address collection by unchecking the 
checkboxes
Incoming mail messages
Outgoing mail messages
Newsgroup messages
Limit collected address book....

I removed the pref because the checkbox associated with it was removed from the 
UI( attachment 34542 [details]), so that we could make the address book panel  fit in the 
preferences window. I was trying to remove it from the backend because the user 
cannot change it from the UI.

Comment 44

18 years ago
Assuming jglick intended for this to be the case, I don't have any problem
removing the pref.  Instead of one simple click to turn it on and off the user
now has to do a few more. If this is the only way we can save space, I think
it's ok as long as the new code is going to work without this pref.  From
looking at the patch it looks like we short circuit some code if they've chosen
not to collect addresses.  I can't see the rest of the code from the patch, but
are we going to have any performance loss when we remove this?
does unchecking all those check boxes have the same effect and disabling that
pref?  

what about the existing users who simply disabled the global pref, but didn't
disable the other prefs?  will mozilla now start collecting addresses on them?
(Assignee)

Comment 46

18 years ago
Unchecking these checkboxes
Incoming mail messages
Outgoing mail messages
Newsgroup messages
should have the same effect as disabling the pref "mail.collect_email_address".

> will mozilla now start collecting addresses on them?
I guess so. Let me see if I can fix this.

Another alternative is try and keep this preference if we can fit it in the 
panel.
Jennifer, is it possible?
(Assignee)

Comment 47

18 years ago
I think it is better to keep the preference. I have a screen shot of the 
Addressing panel. i am going to attach it.
Jennifer what do you think. 
(Assignee)

Comment 48

18 years ago
Created attachment 34750 [details]
screen shot of the Addressing panel

Comment 49

18 years ago
Removed the "Enable..." checkbox was on purpose. Not only to save space, but to 
also make a more clean and straightforward UI. Why should the user have to first 
enable the feature? Checking Incoming, Outgoing or News is what enables/disables 
the feature.  

When this feature was first added, all we had was the ability enable/disable the 
feature, hence the Enable checkbox. Later, we added the ability for users to 
change which items they wanted added. The other items got inserted below the 
enable checkbox instead of reworking the dialog.

Is it possible to make it work this way?

Comment 50

18 years ago
I think it looks sloppy to have to enable the feature and then enable which 
parts of the feature you want below it.  Directly enabling which 
features you want is better.  Is it possible to do it this way?

Also, "Limit..." checkbox should not be grouped with Incoming, Outgoing, News 
items as it is separate from those items.

Comment 51

18 years ago
One of the previous patches does it this way so if that is what you prefer then
I say we should do that.  One side effect will be that people who used older
versions and had the collect addresses disabled (but incoming or outgoing
checked) will start collecting addresses.  But since most people probably didn't
change the default, which was on, that won't be too much of an issue.

Comment 52

18 years ago
Most users never changed the pref settings, plus current usage of the product is 
still very low. Lets fix this the right way before we get our big customer 
following. ;-)
(Assignee)

Comment 53

18 years ago
If that is the case attachment 34617 [details] [diff] [review] and attachment 34700 [details] [diff] [review] should fix the 
problem. And we dhould release note this. 
Is there a bug for release notes where I can add this info.

Comment 54

18 years ago
Use Bugscape bug 4915 for release note items
(Assignee)

Comment 55

18 years ago
Seth, can I get the sr back on the patches :-)
srilatha, please attach a complete and final patch for review.  
(Assignee)

Comment 57

18 years ago
Created attachment 34855 [details] [diff] [review]
Hopefully final patch
since that pref is being removed, you should also remove it from mailnews.js
(Assignee)

Comment 59

18 years ago
Created attachment 34873 [details] [diff] [review]
mail.collect_email_address removed from mailnews.js
assuming the wording is alright with the wordsmiths and the spec, sr=sspitzer.
(Assignee)

Comment 61

18 years ago
fix checked in
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago18 years ago
Resolution: --- → FIXED
(Assignee)

Comment 62

18 years ago
*** Bug 81533 has been marked as a duplicate of this bug. ***

Comment 63

18 years ago
Verified with 2001051704 Wins build.
Status: RESOLVED → VERIFIED
No longer blocks: 80392
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.