Closed
Bug 943096
Opened 11 years ago
Closed 10 years ago
Search subfolders checkbox is always disabled in Search Messages dialog
Categories
(SeaMonkey :: MailNews: Message Display, defect)
SeaMonkey
MailNews: Message Display
Tracking
(seamonkey2.23 wontfix, seamonkey2.24 affected, seamonkey2.25 affected, seamonkey2.26 fixed)
RESOLVED
FIXED
seamonkey2.26
People
(Reporter: iannbugzilla, Assigned: iannbugzilla)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 2 obsolete files)
2.52 KB,
patch
|
iannbugzilla
:
review+
philip.chee
:
approval-comm-aurora+
philip.chee
:
approval-comm-beta+
|
Details | Diff | Splinter Review |
STR 1. Configure mail for an IMAP account that has subfolders 2. Select a folder that has subfolders 3. Open the Search Message dialog from Advanced button, Tools menu or keyboard shortcut 4. Look at Search subfolders checkbox Expected Result 1. Checkbox is enabled Actual Result 1. Checkbox is disabled This has probably been happening since the landing of the patch from bug 59049.
The UpdateSubFolder function is expecting a URI rather than a folder. I guess there are a number of options, either pass a URI, get the function to check or get all callers to pass in a folder. For the moment I have selected the first option.
Attachment #8338074 -
Flags: review?(neil)
Comment 2•10 years ago
|
||
Comment on attachment 8338074 [details] [diff] [review] bug_943096.diff selectFolder calls updateSearchFolderPicker calls UpdateSubFolder so you don't actually need to call it from searchOnLoad. (In fact you don't even need a separate UpdateSubFolder or updateSearchLocalSystem function.)
Attachment #8338074 -
Flags: review?(neil) → review-
This patch makes the suggested changes. This leaves a var folder = GetMsgFolderFromUri(folderURI, true) followed by gMsgFolderSelected = GetMsgFolderFromUri(folderURI) but I am unsure whether the check for folder.hasSubFolders means that we could just use gMsgFolderSelected instead
Attachment #8338074 -
Attachment is obsolete: true
Attachment #8345253 -
Flags: review?(neil)
Comment 4•10 years ago
|
||
(In reply to Ian Neal from comment #3) > This leaves a var folder = GetMsgFolderFromUri(folderURI, true) followed by > gMsgFolderSelected = GetMsgFolderFromUri(folderURI) but I am unsure whether > the check for folder.hasSubFolders means that we could just use > gMsgFolderSelected instead If it's good enough for SearchLocalSystem then it's good enough for SearchSubFolders.
Comment 5•10 years ago
|
||
Ian: if you have the time, could you check if merging your latest patch with attachment 819430 [details] [diff] [review] fixes bug 112578 too (including persist-over-restart)? (I'm still hunting.)
See Also: 112578 →
Comment 6•10 years ago
|
||
(In reply to comment #2) > selectFolder calls updateSearchFolderPicker calls UpdateSubFolder so you > don't actually need to call it from searchOnLoad. (In fact you don't even > need a separate UpdateSubFolder or updateSearchLocalSystem function.) Oops. I added updateSearchLocalSystem and it's completely necessary ;-)
Comment 7•10 years ago
|
||
Comment on attachment 8345253 [details] [diff] [review] Patch with no UpdateSubFolder function >+ var folder = GetMsgFolderFromUri(folderURI, true); >+ document.getElementById("checkSearchSubFolders").disabled = (!folder || !folder.hasSubFolders); >+ > // use the URI to get the real folder > gMsgFolderSelected = GetMsgFolderFromUri(folderURI); > I would just swap these lines around so that you can use gMsgFolderSelected and you don't need to null-check (but you might want to wrap the line or split it into two, compare with menuSearchLocalSystem). r=me with that fixed.
Attachment #8345253 -
Flags: review?(neil) → review+
Carrying forward r+
Attachment #8345253 -
Attachment is obsolete: true
Attachment #8355064 -
Flags: review+
Comment on attachment 8355064 [details] [diff] [review] Patch with less GetMsgFolderFromUri [Checked in: Comment 9] http://hg.mozilla.org/comm-central/rev/0cc1fba46d56
Attachment #8355064 -
Attachment description: Patch with less GetMsgFolderFromUri → Patch with less GetMsgFolderFromUri [Checked in: Comment 9]
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-seamonkey2.23:
--- → wontfix
status-seamonkey2.24:
--- → affected
status-seamonkey2.25:
--- → affected
status-seamonkey2.26:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.26
Assignee | ||
Comment 10•10 years ago
|
||
Comment on attachment 8355064 [details] [diff] [review] Patch with less GetMsgFolderFromUri [Checked in: Comment 9] [Approval Request Comment] Regression caused by (bug #): 59049 User impact if declined: User cannot search sub folders easily Testing completed (on m-c, etc.): on c-c Risk to taking this patch (and alternatives if risky): low String changes made by this patch: none
Attachment #8355064 -
Flags: approval-comm-beta?
Attachment #8355064 -
Flags: approval-comm-aurora?
Comment 11•10 years ago
|
||
Comment on attachment 8355064 [details] [diff] [review] Patch with less GetMsgFolderFromUri [Checked in: Comment 9] a=me for comm-aurora and comm-beta
Attachment #8355064 -
Flags: approval-comm-beta?
Attachment #8355064 -
Flags: approval-comm-beta+
Attachment #8355064 -
Flags: approval-comm-aurora?
Attachment #8355064 -
Flags: approval-comm-aurora+
You need to log in
before you can comment on or make changes to this bug.
Description
•