Closed Bug 78891 Opened 23 years ago Closed 23 years ago

folder properties dialog had both offline and download tabs

Categories

(SeaMonkey :: MailNews: Message Display, defect)

x86
All
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9.1

People

(Reporter: Bienvenu, Assigned: doronr)

References

(Blocks 1 open bug, )

Details

(Whiteboard: r=hwarra, needs sr)

Attachments

(9 files)

I believe the download tab is redundant and should be removed. All it contains is the "select this folder for download" setting, which the offline tab also has.
http://www.mozilla.org/mailnews/specs/offline/#Property is the spec note, there is no "Character Coding" tab in the spec, but there is in our app.
Right, there should be a Character Coding tab also.
Could we make the download now button _not_ span the entire panel, and perhaps sit next to the check box instead of below it?
It definitely shouldn't be the whole width - I don't care about the placement so much. I think 4.x had it below.
It would be nice to get rid of both group boxes also if possible, they aren't necessary since the whole tab is "Offline" stuff.
Reassign to Mohanb
Assignee: dianesun → mohanb
Taking...I've got a fix.
Assignee: mohanb → hwaara
Keywords: patch
OS: Windows 2000 → All
My fix for bug 67615 will fix this.
Priority: -- → P3
QA Contact: esther → gchan
*** Bug 79207 has been marked as a duplicate of this bug. ***
Reassign this to doron as per IRC discussion.
Assignee: hwaara → doronr
Priority: P3 → --
nominating nsbeta1 and mozilla0.9.1. It would be great if you could fix this Doron.
ok, i have a rude version that just needs some to be made nicer on the eye. However: http://www.mozilla.org/mailnews/specs/offline/images/NewsFolder1.gif Currently this does not exist...
just noticed, this exists in the mailnews account settings for newsgroups (Offline & Diskspace). Should this bug encompass that too?
Attached patch patch, needs some polish (deleted) — Splinter Review
patch added, cleans up also some tabbing. if you need a diff -wu tell me. the patch is not final, the offline tab still needs work.
going to wait for 71128 to get checked in before I clean up the offline tab
http://www.nexgenmedia.net/mailfolderpref1.png http://www.nexgenmedia.net/mailfolderpref2.png Preliminary look, offline needs a bit of polish still. The biggest change suggestedmy mpt is that the checkbox in general be turned around ("Apply default to all messages" became "Allow messages to specify other encodings"). I alsochanged the backend js for this. If mailnews people disagree, I can change this back (I understand jglick has veto powers on mail UI :). I feel confident this can make 0.9.1
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.1
ok, the offline tab does not need any more work, so this looks good for me. Any comments or r=/sr=?
Keywords: patch
Attached patch new patch - no tab changes (deleted) — Splinter Review
Attached patch new patch - removes evil tabs (deleted) — Splinter Review
fixed a small alignment issue. 2 patches - the first is the code change, the 2nd includes a total tab removal of folderProps.js, which should be checked in.
anyone for r/sr or should i push this to 0.9.2?
Keywords: review
I can review this after the trunk freeze. Remind me by then, if you still need it.
Naoki and Jennifer, how do you feel about the screen shots? Even if we don't change the character coding, getting rid of the two offline tabs would be a big improvement that I would love to see in 0.9.1.
There are several screen shots (links and an attachment). Which one is the final?
Attached patch in 71128 has a) Download tab removed. b) Charset Coding tab intact. Only change needed is "remove offline tab in case of POP server".
I am not clear about relation of this bug and bug 71128. Bug 71128 does not have screen shots. I want to see a screen shot of the latest proposed change.
http://www.nexgenmedia.net/mailfolderpref1.png http://www.nexgenmedia.net/mailfolderpref2.png this reflects the current look of the mailfolder properties with the attached patch. As for bug 71128, it renames some files I modified, that is the relation. So either, this gets checked in and 71128 reflects this, or 71128 combines these changes with its own.
* Charset coding is merged into "General" (I thought there was a separate bug for that). It looks fine to me. * The string for the checkbox changed. "Allow messages to speficy other encodings" This is not good, please do not change it and use the orignal string unchanged. "Apply default to all messages (ignore charset specified by MIME header)"
I don't agree with you Nhotta. The wording change is per mpt's proposal and if I remember correctly, jglick agreed that the new label was good in another bug (forgot about which). I am reviewing doron's patch, so I find it quite inappropriate to object against wording changes done - and available for the publich 12 days ago.
Jglick, as discussed in other bugs merged with this - we should also merge the Character Encoding tab since it didn't make sense to have a full tab for only one item. I can dig up a bug# for you if you don't believe me.
>The wording change is per mpt's proposal Can I see the proposal? I haven't received complaints about the current working sicne we released NS6. I don't think we need to rash to change the wording for 0.9.1, and it affects localization too.
>we should also merge the Character Encoding tab I see that already merged. http://www.nexgenmedia.net/mailfolderpref1.png
I reviewed the attached patch with doron via IRC. He will not change the wording, as per nhotta.
*** Bug 67615 has been marked as a duplicate of this bug. ***
*** Bug 67612 has been marked as a duplicate of this bug. ***
Merging of character coding into General tab: yeah i remember the bug (although i can't seem to find it at the moment). Just though it was being covered in the other bug instead of this one (so i didn't want to confuse folks). As for wording, what is this checkbox actually supposed to do? The menu item allows users to select a character coding to be used for all the messages in the particular folder? Correct? And the checkbox allows individual messages to override the coding selected in the menu item? Is that correct?
>The menu item allows users to select a character coding to be used for all the > messages in the particular folder? Correct? The menu set a default charset which is used when a charset is not specified in the message. The default applies to the all messages in the folder. >And the checkbox allows individual messages to >override the coding selected in the menu item? No, override option ignores the charset specified in the message and force to use the default charset for any messages in the folder.
Attached patch another patch, another day (deleted) — Splinter Review
Yep, as per IRC. Everything looks good to me except for this: -<!ENTITY folderProps.name.label "Name:"> +<!ENTITY folderProps.name.label "Name:"> Why did you add a tab there? r=hwaara. Good job, doron!
new patch removed the menu item as well, and reverts back the checkbox text. The only wording change is General Information -> General. http://www.nexgenmedia.net/mailfolderpref1.png http://www.nexgenmedia.net/mailfolderpref2.png have been updated with the newest look. As for the tab, not worth readding a new patch (perhaps the one who will cvs commit this can remove it).
> Can I see the proposal? Proposal: * Change the checkbox which says `Apply default to all messages (ignore charset specified by MIME header)' (off by default) to one which says `Allow messages to specify other encodings' (on by default). Advantages: * More naturally fits with humans' understanding of how the option works (as Jennifer's description demonstrated -- she was perfectly correct, it's just that the current checkbox presents the option backwards). * Avoids using `charset', which isn't a word. * Avoids using `MIME header', since even those people who know what an encoding is probably won't know what a MIME header is. (A decapitated street performer, perhaps?) * Is consistent in wording with the current `Allow pages to specify other fonts', future `Allow pages to specify other colors' and `Allow pages to specify other styles', and other similarly functioning checkboxes, making the UI more consistent throughout Mozilla. Disadvantages: * Would need to be checked in by someone who does not work for Netscape, because of Netscape's current localization freeze. > I haven't received complaints about the current > working sicne we released NS6. Maybe that's because, if you'll pardon me for saying so, the number of people who use NS6 for e-mail is very small indeed? The other bug which people can't find, about reducing the number of tabs in this window, is bug 67615. It has been marked as a duplicate of this one because it's being dealt with in the same patch.
please drop 'other' globally it adds no value. A document should be able accidentally to specify the same value too. And specify sucks, perhaps 'suggest', 'determine', 'select', 'indicate'
I've been contacted by netscape people who prefer to keep word changes to a minimum. If you think the change is indeed checked in, i'll notify them about this. Or, we check in without the change and change the text/js functionality after the trunk is opened.
I posted the proposal for folder character coding to mozilla i18n newsgroup.
UI Freeze for 0.9.1 was 5/8/01. Any changes to UI at this stage severly our ability to deliver localised builds on time. I reccomend that you check in with "General Information" not "General" and then make the worging change in the trunk after 0.9.1 branches
i suggest the following. Someone gives this an sr=, and I will create a patch that removes any wording changes for checkin. I am just waiting for an sr=, as I might have to change some code perhaps for the sr.
Whiteboard: needs sr
Ok, let's get this straight. There is *no* UI wording freeze for Mozilla 0.9.1, for people not employed by Netscape (which Doron isn't, at least not last time I checked). If you have the URL of an announcement to the contrary from one of the drivers@mozilla.org, post it here; otherwise, please stop trying to inconvenience people who don't work for Netscape by getting them to split up perfectly valid patches into time-consuming little bits. (If Doron has nothing better to do, that's fine; but I think he'd probably rather be fixing more bugs.) If you want a UI wording freeze that affects Mozilla, all you need to do is discuss it with the drivers@mozilla.org, and get them to announce it to (at least) the n.p.m.seamonkey, n.p.m.ui, and n.p.m.l10n groups with a reasonable amount of notice.
I have to ask a favor. 0.9.1 is coming to an end and this bug probably shouldn't be considered a blocker. There's disagreement over what the wording should be and whether or not we should have multiple tabs on this dialog. I think we all agree that we don't want an offline AND download tab on this dialog. Doron, would it be possible to just fix that (which is what this bug is originally about anyway) and let the wording and dialog configuration debate carry over to 0.9.2 where no one is up against a deadline? I really would like to only see one offline panel in 0.9.1, so it would be really cool if we could all agree to disagree for a little while longer and fix the problem that I think we all agree on in the short term.
Blocks: 71128
Attached patch no wording changes patch (deleted) — Splinter Review
new patch that honours l10n. No wording changes. The new patch also contains numerous tab fixes. Only difference is "General" reverted back to "General Information". sspitzer said he would sr= this now that it has no l10n changes.
Whiteboard: needs sr → r=hwarra, needs sr
thanks for posting that second patch. I'm off to test and super review this now.
great, thanks! If it passes the sr= process, could you check it in for me?
yes, I'll land it for you when I'm done testing and reviewing.
overall, good work. this type of cleanup and polish of broken UI really helps the product out. some issues with your patch: 1) the way you're generating the title for the dialog is not correct. you need to use a string bundle for that, instead of a entity and string replacing. for 0.9.1 (because of the UI freeze) we should just leave the title as it was. we can log a 0.9.2 bug to make the title dynamic (and to use string bundles). 2) "View | Folder Character Coding..." is broken. it will open up the properties dialog and the general tab will be blank. I've got the fix for that. but looking at your previous patch (http://bugzilla.mozilla.org/showattachment.cgi?attach_id=35681) you've fixed this problem by removing "View | Folder Character Coding..." altogether. http://www.mozilla.org/mailnews/specs/threepane/MailMenus.html still has the element, so I'm going to follow the spec. jglick, there is no spec for this dialog. can you add something to the spec http://www.mozilla.org/mailnews/specs/threepane/index.hml#Folders I'll attach two patches. one, a patch based on doron's initial patch with the fix for problem #2, and out any string changes (to minimize the impact on the L18n efforts) the second patch will be one with all the desired string changes (including a fix for problem #1). I'll send mail to danielmc@netscape.com and mcarlson@netscape.com to see if we can get approval for complete string changes, otherwise it will have to wait.
Seth, I think Jennifer should update the spec to remove that redundant menuitem now that the Character Encoding tab will be gone. That a fix doesn't follow the spec might as well be a request to improve the spec, rather than fixing up the patch. In this case, I think we should make all the wording changes and remove the necessary stuff and then update the specs with those changes.
I'll await a comment from jglick (about the change in spec) before I tweak the menuitem. she's more on top of the issues than I am. I think rememeber nhotta and / or kat arguing for it, so I'm following the spec. now to see if I can get approval for some string changes, otherwise they'll have to wait. thanks to doron for understanding why the string changes might have to wait a milestone.
question - can we get rid of the download tab for local folders? Do any of these patches do that? It's non-sensical and confusing for local mail users.
Bienvenu, this fix removes all tabs except for "Offline" and "General".
bienvenu, that will be covered by bug #71128, which will depend on me fixing #79239 which will give us the infrastructure work to fix all these "non offline folders and servers have offline UI elements" bugs. if someone wants to make #79239 a 0.9.1 bug, I can code up the fix.
ah, OK, Seth, I was just asking. Your changes look fine to me, sr=bienvenu - my remaining issues are covered by 71128. Hakan, for local folders, there shouldn't be an Offline/Download tab at all. Selecting a local folder for offline download use doesn't have any meaning.
i'll post a patch that adds the wording changes tomorrow for when we branch. Also will fix the mistake of not using getFormattedString from a stringbundle
I'm going to land the last patch and then open a new bug (on doron) to clean up all the string changes.
hold up, trying a simple fix (suggested by hwaara) to hide the offline tab for local and pop servers...
excellent, thanks to a suggestion by hwaara, the offline tab only shows up when the folder is a nntp or imap folder (hidden for pop3 or none). the goal is still to make this code dynamic (so that we don't have to hard code server types) but this is good for now. new patch on the way.
r=hwaara
great, sr=bienvenu
great! a= asa@mozilla.org for checkin to 0.9.1
>2) "View | Folder Character Coding..." is broken. it will open up the >properties dialog and the general tab will be blank. I've got the fix for >that.but looking at your previous patch >(http://bugzilla.mozilla.org/showattachment.cgi?attach_id=35681) you've fixed >this problem by removing "View | Folder Character Coding..." altogether. >http://www.mozilla.org/mailnews/specs/threepane/MailMenus.html still has the >element, so I'm going to follow the spec. Please removed the menu item from the View menu. I will update the spec to reflect. >jglick, there is no spec for this dialog. can you add something to the spec >http://www.mozilla.org/mailnews/specs/threepane/index.hml#Folders Will do shortly.
I've landed the last patch. next stop, before I close this: 1) remove that menu item from the view menu 2) log new bug on doron for the wording / string bundle changes.
marking fixed. I'll open the two remain problems as new bugs.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
I posted a note in the mozilla i18n newsgroup about removing the menu item from view menu.
Functional in 2001053108 commercial linux build Functional in 2001053108 commercial MacOS build Functional in 2001053108 commercial Windows build The additional functional difference between POP and IMAP folders is also present for all three platforms Standing in For Gary Chan
marking verified based on lrg's comments Download tab removed from all differnt accounts. Offline tab only present in news,webmail,and imap accounts. Offline tab emoved from Pop and Local folders. Checked both classic and modern.
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: