Closed Bug 943096 Opened 7 years ago Closed 7 years ago

Search subfolders checkbox is always disabled in Search Messages dialog

Categories

(SeaMonkey :: MailNews: Message Display, defect)

defect
Not set
normal

Tracking

(seamonkey2.23 wontfix, seamonkey2.24 affected, seamonkey2.25 affected, seamonkey2.26 fixed)

RESOLVED FIXED
seamonkey2.26
Tracking Status
seamonkey2.23 --- wontfix
seamonkey2.24 --- affected
seamonkey2.25 --- affected
seamonkey2.26 --- fixed

People

(Reporter: iann_bugzilla, Assigned: iann_bugzilla)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

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.
Assignee: nobody → iann_bugzilla
Status: NEW → ASSIGNED
Blocks: 112578
Attached patch bug_943096.diff (obsolete) — Splinter Review
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 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)
(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.
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
(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 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: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.26
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 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.