Closed
Bug 340324
Opened 19 years ago
Closed 13 years ago
There is no Local Directory field in settings for RSS account
Categories
(MailNews Core :: Account Manager, defect)
MailNews Core
Account Manager
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 14.0
People
(Reporter: arkady.belousov, Assigned: aceman)
References
Details
Attachments
(1 file, 4 obsolete files)
23.85 KB,
patch
|
aceman
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/4.0 (compatible; MSIE 6.0; Windows 98; ru) Opera 8.54 Build Identifier: http://mozilla-chi.osuosl.org/pub/mozilla.org/thunderbird/releases/1.5.0.4/win32/ru/Thunderbird%20Setup%201.5.0.4.exe TB creates directory for RSS acount very deeply (<default>\Mail\News & Blogs) and there is no options to place content in other (custom) directories. The more so, I see there no relations with name of account, which I create. BTW, there is something wrong with directories for News accounts: if I move directory for News account into other place, then TB beside directory also creates .msf file with same name in PARENT directory (ie, <dir>\<my news account>\ and <dir>\<my news account>.msf). Reproducible: Always
Comment 1•19 years ago
|
||
*** This bug has been marked as a duplicate of 320571 ***
Status: UNCONFIRMED → RESOLVED
Closed: 19 years ago
Resolution: --- → DUPLICATE
Reporter | ||
Comment 2•19 years ago
|
||
> *** This bug has been marked as a duplicate of 320571 *** Sorry, absolutely no relations: I say about missing field in Preferences, whereas bug 320571 discusses nested attachments in forwarded mails.
Status: RESOLVED → UNCONFIRMED
Resolution: DUPLICATE → ---
Comment 3•19 years ago
|
||
You're right, that was the wrong bug. I meant to dupe it to bug 307392. However, I'm now thinking it's not strictly a dupe, only because the "Local Directory" setting doesn't appear in the "Disk Space" tab for other accounts. Incidentally: you can make these changes yourself: 1) close TB, locate the prefs.js file in your profile, and edit it, OR 2) in the Config Editor, use a filter of ".directory" Find the two references to the current path (.directory and .directory-rel) and replace them with the desired path. Then either rename the existing directory, or create the new directory where you want it and move all the files from the old directory to the new one; and restart TB. I don't recommend that you move the directory to somewhere outside of the profile directory, as the .directory-rel preference is a relative path starting at the profile directory.
Status: UNCONFIRMED → NEW
Component: Preferences → Account Manager
Ever confirmed: true
OS: Windows 98 → All
Hardware: PC → All
Summary: There is no Local Directory filed in Preferences for RSS account → There is no Local Directory field in settings for RSS account
Version: unspecified → Trunk
Reporter | ||
Comment 4•19 years ago
|
||
> Incidentally: you can make these changes yourself: > 1) close TB, locate the prefs.js file in your profile, and edit it, OR As I understand, this is wrong way, because prefs.js is overwritten by TB when and as it wish. user.js also doesn't help much, because sometime TB forgets options from there (see my bug 339600 comment #5 - BTW, TB forgets edited options after opening any separate window). > 2) in the Config Editor, use a filter of ".directory" Same trouble - see my bug 339600. > I don't recommend that you move the directory to somewhere outside of the > profile directory, as the .directory-rel preference is a relative path starting > at the profile directory. _All_ of my custom directories I place much higher. For example, TB places its profile into %appdata%\Thunderbird and places profile very, very deeply in there. There I replace content of profiles.ini by "IsRelative=0/Path=%appdata%\TB\DEFAULT". Then I place all other directories in %appdata%\TB (for example, %appdata%\TB\Local). BTW, Firefox scheme is worser - it places profiles.ini into %appdata%\Mozilla\Firefox.
Comment 5•19 years ago
|
||
(In reply to comment #4) > > Incidentally: you can make these changes yourself: > > 1) close TB, locate the prefs.js file in your profile, and edit it, OR > > As I understand, this is wrong way, because prefs.js is overwritten by TB > when and as it wish. It's not as random as that. Some settings might get changed, but the paths are not likely to be. Possibly, since you have this other problem, you might see such a change. OT: It seems to me there's a good chance that your moved-around directories could be the *cause* of the problem in bug 339600. If you haven't properly gotten every setting to point to the correct directories, there could be interactions that make no sense. I suggest creating a new profile, and let the directories exist where TB puts them -- see if you can then reproduce the kinds of problems you're seeing with settings getting overwritten.
Reporter | ||
Comment 6•19 years ago
|
||
> OT: It seems to me there's a good chance that your moved-around directories > could be the *cause* of the problem in bug 339600. If you haven't properly > gotten every setting to point to the correct directories, there could be > interactions that make no sense. All, what I do, I do from TB itself (except copying contents of directory to new place after changing Local Directory field). > I suggest creating a new profile, and let the > directories exist where TB puts them How do this? Will this harm my existing profile? > -- see if you can then reproduce the kinds > of problems you're seeing with settings getting overwritten.
Updated•18 years ago
|
QA Contact: preferences → account-manager
Updated•16 years ago
|
Assignee: mscott → nobody
Comment 7•16 years ago
|
||
(In reply to comment #6) > > I suggest creating a new profile, and let the > > directories exist where TB puts them > > How do this? Will this harm my existing profile? create a new profile using profile manager. start from command line/dos window in thunderbird directory and use thunderbird.exe -P
I can try to expose the Local Directory in the account manager for Feeds. As the backend seems to handle it if done manually.
Assignee: nobody → acelists
Product: Thunderbird → MailNews Core
QA Contact: account-manager → account-manager
I think the Manage subscriptions button is now too close to the Browse button. It is a bit odd. Should I move it somewhere else?
Attachment #591898 -
Flags: ui-review?(bwinton)
Attachment #591898 -
Flags: review?(iann_bugzilla)
Comment 10•13 years ago
|
||
Comment on attachment 591898 [details] [diff] [review] expose field >+++ b/mailnews/extensions/newsblog/content/am-newsblog.xul >@@ -49,18 +49,21 @@ > onload="parent.onPanelLoaded('am-newsblog.xul');"> > > <script type="application/javascript" > src="chrome://messenger/content/AccountManager.js"/> > <script type="application/javascript" > src="chrome://messenger-newsblog/content/am-newsblog.js"/> > <script type="application/javascript" > src="chrome://messenger-newsblog/content/newsblogOverlay.js"/> >+ <script type="application/javascript" >+ src="chrome://messenger/content/amUtils.js"/> > > <dialogheader id="am-newsblog-title" defaultTitle="&accountTitle.label;"/> >+ <stringbundle id="bundle_prefs" src="chrome://messenger/locale/prefs.properties"/> Presumably not needed with the current fix in bug 218439? I agree, it does look a bit odd. I think there is probably a few options (not all mutually exclusive): 1/ Put button into a group box entitled "Subscriptions". 2/ Move button to top of page. 3/ Move the local directory to the disk space/storage page (this would change it for all account types though). 4/ Remove the button, news accounts don't have such a button. There are probably other options too. I do like the idea of doing at least 3 but that definitely needs a ui-review. I have also noticed that the checkboxes should probably be in a group box entitled "Server Settings" to match other account types. For the moment I'll leave the review request pending and see what Blake says wrt UI.
![]() |
Assignee | |
Comment 11•13 years ago
|
||
Thanks. UI review is already requested. I'll remove the stringbundle line after that.
![]() |
Assignee | |
Comment 12•13 years ago
|
||
I added the 'Server Settings' group header. I'd like to add a "Subscribe button" to News but I find it hard to hook it to the Subscribe functions in the main Mail window.
Comment 13•13 years ago
|
||
(In reply to :aceman from comment #12) > I added the 'Server Settings' group header. > I'd like to add a "Subscribe button" to News but I find it hard to hook it > to the Subscribe functions in the main Mail window. If you follow it through, you need to call the Subscribe function in mailCommands.js with the correct argument. Just a matter of making sure that mailCommands.js is within the scope of the account manager page.
![]() |
Assignee | |
Comment 14•13 years ago
|
||
Yes, and that is what I have problem with. I am not yet able to follow the scope of things. If that function is somehow accessible with some window.top.x....Subscribe(). What tool can I use for that? DOM Inspector?
Comment 15•13 years ago
|
||
(In reply to :aceman from comment #14) > Yes, and that is what I have problem with. I am not yet able to follow the > scope of things. If that function is somehow accessible with some > window.top.x....Subscribe(). What tool can I use for that? DOM Inspector? Not sure if there is a tool that does or not. am-newsblog.xul loads newsblogOverlay.js, so perhaps am-server.xul could load mailCommands.js or the relevant functions moved to a file that is already loaded by am-server.xul or AccountManager.xul. Either way it is outside the scope of this bug.
Comment 16•13 years ago
|
||
Comment on attachment 591898 [details] [diff] [review] expose field So, I _really_ don't think people use that field, but it's on all the other account settings, so I guess for consistency we should add it here, too. (Or perhaps we should remove it from all the other ones… ;) ui-r=me. Thanks, Blake.
Attachment #591898 -
Flags: ui-review?(bwinton) → ui-review+
![]() |
Assignee | |
Comment 17•13 years ago
|
||
Bwinton, what about the Manage subscriptions button ?
Comment 18•13 years ago
|
||
(In reply to :aceman from comment #17) > Bwinton, what about the Manage subscriptions button ? Oh, yeah. I didn't really notice it as being that out of place, perhaps because the rest of the settings aren't grouped. (If you want to fix that, feel free, but I won't let it block the ui-r+ if you don't want to.) Moving the Local Directory settings to the Disk Space page seems like a pretty big change, and I don't think we want to go that far just yet. > 1/ Put button into a group box entitled "Subscriptions". Since that would be the only group box, that seems strange. > 2/ Move button to top of page. Enh, all the Manage Identities buttons are at the bottom. > 4/ Remove the button, news accounts don't have such a button. I bet people would complain about this, too. What about putting the Local Directory in a group, a la Local Folders: http://dl.dropbox.com/u/2301433/Screenshots/LocalFolders.png ? At the end, I'ld be happy enough to leave it the way it is, so I'm going to let the ui-r+ stand, but if you wanted to add that group, that would be fine, too. Thanks, Blake.
![]() |
Assignee | |
Comment 19•13 years ago
|
||
Thanks. I have already grouped the other checkboxes to be under "Server settings" as other account types have it. I can group the Local Directory even though it would be the only element in that group (on POP3 Server settings it does not have this grouping). But it would push the button further apart. I'll try it.
Attachment #591898 -
Flags: review?(iann_bugzilla) → review-
![]() |
Assignee | |
Comment 20•13 years ago
|
||
Adds the Server settings group into RSS panel. Also adds Message Storage group into RSS account pane, but also all Server settings panes. It looks quite nice and consistent. It also nicely contains the newsrc path and "empty thrash on exit" checkboxes on all account types. Please check all account types. The only problem is the Advanced button on IMAP account, which does not contain message storage options. The one on POP3 can be considered as such.
Attachment #591898 -
Attachment is obsolete: true
Attachment #598290 -
Flags: ui-review?(bwinton)
Attachment #598290 -
Flags: review?(iann_bugzilla)
Comment 21•13 years ago
|
||
(In reply to :aceman from comment #20) > Created attachment 598290 [details] [diff] [review] > patch, add option groupings > > Adds the Server settings group into RSS panel. Also adds Message Storage > group into RSS account pane, but also all Server settings panes. It looks > quite nice and consistent. It also nicely contains the newsrc path and > "empty thrash on exit" checkboxes on all account types. Please check all > account types. The only problem is the Advanced button on IMAP account, > which does not contain message storage options. The one on POP3 can be > considered as such. Trouble is IMAP and NNTP accounts have a pref panel called "Synchronization & Storage" which is confusing when you have group boxes called "Message Storage"
![]() |
Assignee | |
Comment 22•13 years ago
|
||
That is true. However, that panel has options about which messages to store/delete. The Server settings panel/Local Directory and co. define where they are stored on disk. They are complementary. Maybe it just needs to reword the labels better. Yes, it is possible to add the Message Storage group only to the RSS panel as bwinton suggested. I just found it inconsistent with the other panels, where the same field is not in any group (expect maybe Local Folders).
Comment 23•13 years ago
|
||
For the moment, I would just do the Message Storage group for the RSS panel. Potentially then a spin off bug for dealing with inconsistencies / code duplication / etc.
Comment 24•13 years ago
|
||
Comment on attachment 598290 [details] [diff] [review] patch, add option groupings r- for the moment, missing SeaMonkey locale change
Attachment #598290 -
Flags: review?(iann_bugzilla) → review-
![]() |
Assignee | |
Comment 25•13 years ago
|
||
Let's see what bwinton says before I delete half of the patch :)
Comment 26•13 years ago
|
||
Comment on attachment 598290 [details] [diff] [review] patch, add option groupings Yeah, I'm pretty happy with how this patch looks. The only thing I might suggest is to add a sentence to the top saying "The following is a special account. There are no identities associated with it", like the one in Local Folders. But if you don't want to do that, that's okay, too. :) ui-r=me. Thanks, Blake.
Attachment #598290 -
Flags: ui-review?(bwinton) → ui-review+
![]() |
Assignee | |
Comment 27•13 years ago
|
||
And what about Ian's comment 21? Have you seen all the panels in all account types?
Comment 28•13 years ago
|
||
I haven't, since I don't have a POP3 account, but I'm fine with having different settings for different types of account, and the blogs feel more to me like local folders than like IMAP or POP3 accounts. (Also, we already have a group box called "Message Storage" in the local folders, so it's not like this patch is creating a new problem.) For a longer-term fix, and this might be too big for you to want to tackle, I think we should merge the preferences and account settings, and have them be searchable, so that people will always be able to find the settings they're interested in, no matter where they are. Something like MacOS X's Preferences, or Windows 7's Control Panel.
![]() |
Assignee | |
Comment 29•13 years ago
|
||
I added the string as bwinton wished. I noticed most of the strings in am-serverwithnoidentities.dtd are duplicated in am-server-top.dtd. So I merged them a bit. I don't know if it is OK. The strings and accesskeys are identical in english. Are they identical in other languages? Is this optimization OK?
Attachment #598290 -
Attachment is obsolete: true
Attachment #599783 -
Flags: review?(iann_bugzilla)
Comment 30•13 years ago
|
||
Comment on attachment 599783 [details] [diff] [review] patch v2 >+++ b/mailnews/base/prefs/content/am-server.xul >+ <hbox align="end"> >+ <vbox align="start" flex="1" id="exitHandlingBox="> >+ <checkbox hidefor="pop3,nntp,movemail" wsm_persist="true" id="imap.cleanupInboxOnExit" >+ label="&expungeOnExit.label;" >+ accesskey="&expungeOnExit.accesskey;" >+ prefattribute="value" >+ prefstring="mail.server.%serverkey%.cleanup_inbox_on_exit"/> Nit: where you are touching code that has an element which is over multiple lines, could you have one attribute per line. >+ <checkbox hidefor="nntp" wsm_persist="true" id="server.emptyTrashOnExit" >+ label="&emptyTrashOnExit.label;" >+ accesskey="&emptyTrashOnExit.accesskey;" >+ prefattribute="value" >+ prefstring="mail.server.%serverkey%.empty_trash_on_exit"/> Nit: same again here. >+ </vbox> >+ <button label="&advancedButton.label;" >+ accesskey="&advancedButton.accesskey;" >+ oncommand="onAdvanced();" >+ wsm_persist="true" id="server.advancedbutton" >+ prefstring="mail.server.%serverkey%.advanced.disable" >+ hidefor="nntp,movemail"/> Nit: and here. >+ </hbox> >+ >+ <vbox hidefor="imap,pop3,movemail"> >+ <label value="&newsrcFilePath.label;" control="nntp.newsrcFilePath"/> >+ <hbox align="center"> >+ <textbox readonly="true" wsm_persist="true" flex="1" id="nntp.newsrcFilePath" >+ datatype="nsILocalFile" >+ prefstring="mail.server.%serverkey%.newsrc.file" class="uri-element"/> Nit: etc >+ <button id="browseForNewsrc" label="&browseNewsrc.label;" filepickertitle="&newsrcPicker.label;" >+ accesskey="&browseNewsrc.accesskey;" oncommand="BrowseForNewsrc()"/> Nit: and here, also have ; after the call in oncommand. >+ </hbox> > </vbox> >+ <separator class="thin"/> >+ >+ <vbox> >+ <label value="&localPath.label;" control="server.localPath"/> >+ <hbox align="center"> >+ <textbox readonly="true" wsm_persist="true" flex="1" id="server.localPath" datatype="nsILocalFile" >+ prefstring="mail.server.%serverkey%.directory" class="uri-element"/> Nit: and here >+ <button id="browseForLocalFolder" label="&browseFolder.label;" filepickertitle="&localFolderPicker.label;" >+ accesskey="&browseFolder.accesskey;" oncommand="BrowseForLocalFolders()"/> Nit: and here, also have ; after the call in oncommand. >+ </hbox> >+ </vbox> >+ > </groupbox> > > <separator class="thin"/> >+ >+ <hbox hidefor="imap,pop3,movemail" align="center" valign="middle" iscontrolcontainer="true"> >+ <label value="&serverDefaultCharset.label;" control="nntp.charset"/> >+ <menulist hidable="true" hidefor="imap,pop3,movemail" wsm_persist="true" id="nntp.charset" >+ ref="NC:DecodersRoot" datasources="rdf:charset-menu" >+ preftype="string" prefstring="mail.server.%serverkey%.charset"> Nit: and here >+++ b/mailnews/base/prefs/content/am-serverwithnoidentities.xul >+<!DOCTYPE page [ >+<!ENTITY % accountNoIdentDTD SYSTEM "chrome://messenger/locale/am-serverwithnoidentities.dtd" >%accountNoIdentDTD; >+<!ENTITY % trashDTD SYSTEM "chrome://messenger/locale/am-server-top.dtd">%trashDTD; Nit: need a better name for the entity, accountServerTopDTD perhaps? >+++ b/mailnews/extensions/newsblog/content/am-newsblog.xul >@@ -36,86 +36,114 @@ > > <?xml-stylesheet href="chrome://messenger/skin/accountManage.css" type="text/css"?> > > <!DOCTYPE page [ > <!ENTITY % newsblogDTD SYSTEM "chrome://messenger-newsblog/locale/am-newsblog.dtd" > > %newsblogDTD; > <!ENTITY % accountNoIdentDTD SYSTEM "chrome://messenger/locale/am-serverwithnoidentities.dtd" > > %accountNoIdentDTD; >+<!ENTITY % trashDTD SYSTEM "chrome://messenger/locale/am-server-top.dtd">%trashDTD; Nit: need a better name for the entity, accountServerTopDTD perhaps? >+ <vbox> >+ <label value="&localPath.label;" control="server.localPath"/> >+ <hbox align="center"> >+ <textbox readonly="true" wsm_persist="true" flex="1" id="server.localPath" datatype="nsILocalFile" >+ prefstring="mail.server.%serverkey%.directory" class="uri-element"/> Nit: where you are touching code that has an element which is over multiple lines, could you have one attribute per line. >+ <button id="browseForLocalFolder" label="&browseFolder.label;" filepickertitle="&localFolderPicker.label;" >+ accesskey="&browseFolder.accesskey;" oncommand="BrowseForLocalFolders()"/> Nit: and here, also have ; after the call in oncommand. r- due to the number of nits.
Attachment #599783 -
Flags: review?(iann_bugzilla) → review-
![]() |
Assignee | |
Comment 31•13 years ago
|
||
Thanks, fixed.
Attachment #599783 -
Attachment is obsolete: true
Attachment #601624 -
Flags: review?(iann_bugzilla)
Attachment #601624 -
Flags: review?(iann_bugzilla) → review+
Attachment #601624 -
Flags: review?(mbanner)
Comment 32•13 years ago
|
||
Comment on attachment 601624 [details] [diff] [review] patch v3 Review of attachment 601624 [details] [diff] [review]: ----------------------------------------------------------------- ::: mail/locales/en-US/chrome/messenger/am-server-top.dtd @@ +78,5 @@ > <!ENTITY browseNewsrc.label "Browse…"> > <!ENTITY browseNewsrc.accesskey "e"> > + > +<!ENTITY accountTitle.label "Account Settings"> > +<!ENTITY accountSettingsDesc.label "The following is a special account. There are no identities associated with it."> nit: just use one space please. The backend hides the double space anyway, and I believe the UI generally only uses one space.
Attachment #601624 -
Flags: review?(mbanner) → review+
![]() |
Assignee | |
Comment 33•13 years ago
|
||
Thanks, space fixed (it was there before, I just moved it). r=IanN,standard8.
Attachment #601624 -
Attachment is obsolete: true
Attachment #610569 -
Flags: review+
Keywords: checkin-needed
Comment 34•13 years ago
|
||
http://hg.mozilla.org/comm-central/rev/085ffc3c8da5
Status: ASSIGNED → RESOLVED
Closed: 19 years ago → 13 years ago
Flags: in-testsuite?
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 14.0
You need to log in
before you can comment on or make changes to this bug.
Description
•