User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:29.0) Gecko/20100101 Firefox/29.0 SeaMonkey/2.26a2 (Beta/Release) Build ID: 20140312013001 Steps to reproduce: add new message filter or edit existing one with action copy or move to folder Actual results: folder list for target of action stays empty. Existing filters keep working. Only GUI for selection is broken. applies to pop3, IMAP and news-Accounts Probably related error messages: Error: : Component returned failure code: 0x80550005 [nsIMsgFolder.msgDatabase] Source File: chrome://messenger/content/mailWidgets.xml Line: 1938 Error: this.mFilterList is null Source File: chrome://messenger/content/FilterListDialog.js Line: 79 Expected results: List of available target folders should be visible.

Target window for 'Move' is empty for an existing filter on POP3 account SM-Trunk Linux x86_64. Regression window: Last good: 2014-01-27 04:05:00 PST http://hg.mozilla.org/mozilla-central/rev/611698b4a246 http://hg.mozilla.org/comm-central//rev/5a029594d5f0 First bad: 2014-01-28 00:32:00 PST http://hg.mozilla.org/mozilla-central/rev/4da3e21a0e5f http://hg.mozilla.org/comm-central//rev/dc59ad3ee2f7

Mhm, better to give a picture as example

No problem in Thunderbird 30.0a 20140316235303 GMT. Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:30.0) Gecko/20100101 Thunderbird/30.0a1

The only thing suspicious is Bug 878805

Backout of the 4 patches of Bug 878805 was not possible, except the last one. But. *g* First i removed all parts not belonging to SM, then the rest which still would fail to backout, then i renamed the patches alphabetically so that my script would back them out in the correct order and then build a new SM while backing out the modified patches. Well, the problem was gone. There is a great probability now that the culprit is within the patches of Bug 878805.

Setting dependency on Bug 878805 - Check UI consistency across all Thunderbird folderpickers

(In reply to Philip Chee via email) I think this would be a great time to unfork FilterListDialog.js (suite's is radically different and rdf based). This would mean unforking /suite/mailnews/search/SearchDialog.xul and /suite/mailnews/search/FilterListDialog.xul and getting rid of the above plus /suite/mailnews/msgFolderPickerOverlay.xul and /suite/mailnews/search/FilterListDialog.js and /suite/locales/en-US/chrome/mailnews/msgFolderPickerOverlay.dtd and /mailnews/base/content/msgFolderPickerOverlay.js (suite is the only user) See bug 507676 - seems like churn when suite could simply unfork.

Oh I agree, but I have totally no idea how the old rdf or the new filterlist code works. I guess this is HELPWANTED, or rather bug 507676 is.

all you have to do is copy /mail/base/content/FilterListDialog.js /mail/base/content/FilterListDialog.xul /mail/base/content/SearchDialog.js /mail/base/content/SearchDialog.xul to /suite/mailnews/search as a start and build, see if there's something else that might be needed, then get rid of unused msgFolderPickerOverlay files. i don't know how you pick which /mail modules suite uses, build time maybe to get /mail stuff automatically.

(In reply to alta88 from comment #11) > i don't know how you pick which /mail modules suite uses, build time maybe > to get /mail stuff automatically. SeaMonkey uses nothing from /mail/ Mail is Thunderbird UI. We do share /mailnews/. SeaMonkey Mail code is in /suite/mailnews/ So I did a little digging. The Filter Editor lives in /mailnews/ so we should have exactly the same Filter Editor. But the Folder Picker widget is blank in SeaMonkey. function onEditFilter() //SeaMonkey version var selectedFilter = currentFilter(); var curFilterList = currentFilterList(); var args = {filter: selectedFilter, filterList: curFilterList}; window.openDialog("chrome://messenger/content/FilterEditor.xul", "FilterEditor", "chrome,modal,titlebar,resizable,centerscreen", args); function onEditFilter() //Thunderbird version let selectedFilter = currentFilter(); if (!selectedFilter) return; let args = {filter: selectedFilter, filterList: gCurrentFilterList}; window.openDialog("chrome://messenger/content/FilterEditor.xul", "FilterEditor", "chrome,modal,titlebar,resizable,centerscreen", args); So I don't see any substantive differences.

(In reply to Philip Chee from comment #12) > (In reply to alta88 from comment #11) > > > i don't know how you pick which /mail modules suite uses, build time maybe > > to get /mail stuff automatically. > SeaMonkey uses nothing from /mail/ Mail is Thunderbird UI. We do share > /mailnews/. SeaMonkey Mail code is in /suite/mailnews/ > somewhere along the way, the 4 files in comment 11 forked. who knows how they ended up in /suite/mailnews/search/ but the Tb versions have had a lot of work done and thus the suggestion to unfork/copy them to /suite/mailnews/search/. to keep them synced, you can copy at build time. up to you. > So I did a little digging. The Filter Editor lives in /mailnews/ so we > should have exactly the same Filter Editor. But the Folder Picker widget is > blank in SeaMonkey. > you're digging in the wrong place. clearly shared code won't be the problem if it works in Tb. the error is due to suite's treeview implementation in menulists which requires a non null folder at setup time, and doesn't handle null (ie the folder picker in Tb puts up the Choose Folder label due to design or the folder doesn't exist). Tb doesn't use the treeview for folder menulists. the sole remaining folder treeview is in the saved search/virtual folder multifolder select dialog.

This affects our next beta, so either we should backout the original patch or try to rather fix this bug here quickly, for example by unforking (See Comment 11 and Comment 13), not sure how easy that is.

this patch copies /mail/base/content/FilterListDialog.js /mail/base/content/FilterListDialog.xul to suite/mailnews/search/. the .js file imports only shared modules; the .xul file's script/dtd are the same and there are additional existing css files included (from overlays that are removed). we'll see if it's this easy..

I assume copying of the TB files is a quick fix for the nearest release of SM. But actual unforking would be to merge the 2 copies into /mailnews/base/search . I would support that. But it can be done in a new bug after branching for TB32. Not merging the files would keep them forked and allow for new differences in the future. But maybe I didn't understand and that is what is wanted?

This Problem I created a Bug for Yesterday or day before For Mac OSX.9.2 and SeaMonkey 26 Beta 1 for same problem. Had to revert to Version 25 Beta in order for ability to create Message filters

Comment on attachment 8400316 [details] [diff] [review] suite.patch Not sure why, but it does not work this way. To get the patch to apply, I had to copy the FilterListDialog.js file from mail/... again as there were conflicts on patch apply. When trying to open the filter dialog, I get this error message: XML Parsing Error: undefined entity Location: chrome://messenger/content/FilterListDialog.xul Line Number 14, Column 1:<window id="filterListDialog" ^ MDN says this can be caused by a failure to load a dtd file. Opening chrome://messenger/locale/FilterListDialog.dtd in the browser works fine though.

MDN reference: https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/window#Note

Actually copying mail/locales/en-US/chrome/messenger/FilterListDialog.dtd to suite/locales/en-US/chrome/mailnews/FilterListDialog.dtd fixes this problem (of course this would work for trunk only). But then the folder list still seems to be empty..

(In reply to Frank Wein [:mcsmurf] from comment #22) > Actually copying mail/locales/en-US/chrome/messenger/FilterListDialog.dtd to > suite/locales/en-US/chrome/mailnews/FilterListDialog.dtd fixes this problem Right. The issue was window.title vs filterListDialog.title. The "undefined entity" error message told you that (unfortunately the line and column information given in the error message is often useless/wrong). (In reply to Frank Wein [:mcsmurf] from comment #22) > But then the folder list still seems to be empty.. I wouldn't be surprised if our implementations of other parts that the code is referring to are quite different. This is also the reason why I felt copying anything from mail/ here would not be advised for any quick branch fix, which should be our priority. There I think we should go the back-out route. For trunk I agree copy/adapt might be a good strategy, provided we test it thoroughly (and get it to work in the first place, but that is just a matter of time).

(In reply to Jens Hatlak (:InvisibleSmiley) from comment #23) > (In reply to Frank Wein [:mcsmurf] from comment #22) > > But then the folder list still seems to be empty.. > > I wouldn't be surprised if our implementations of other parts that the code > is referring to are quite different. This is also the reason why I felt > copying anything from mail/ here would not be advised for any quick branch > fix, which should be our priority. There I think we should go the back-out > route. For trunk I agree copy/adapt might be a good strategy, provided we > test it thoroughly (and get it to work in the first place, but that is just > a matter of time). The filter backend should be the same as TB's in /mailnews/base/search, only the frontend filter list dialog is forked. The filters should also be stored in the same place. So I don't think the implementations are that different, there should be just some small tweak to do.

aceman: So would backing out be acceptable for TB 29 at least? Next big TB release is 31 afaik. Or we need come up with a quick solution for this for SeaMonkey to fix what you mentioned in Comment 13. Even if we want to copy all the files over, fixing the localization strings is not possible for beta anymore. For aurora it may be doable (late-l10n).

I have not written comment 13 :) Backing out of TB29 beta is a question from standard8, but as long as you find the needed patches (there were some fixes on top of bug 878805) it should be possible. The patch itself is a visual polish, not a critical feature. It will do no harm if it is pulled out of TB29 beta that never gets released. But alta88 is the author so he should comment on it. Can the differing strings be fixed on the TB side? Is there something more than the 1 string with differing ID from comment 23? But the filterListDialog.title is probably the better name to keep.

The failure of the simple copy patch was due to, of course, strings. The following patch works (suite has actually been built/tested a bit with it). As for backing out of Tb29, there have been some patches on top of bug 878805. The ramifications are unclear, likely minor, but it would seem just backing out of 29 is doable, as long as it remains in 30/31.

Comment on attachment 8407519 [details] [diff] [review] suite.patch >-<!ENTITY filterListDialog.title "Message Filters"> >+<!ENTITY window.title "Message Filters"> As mentioned in previous comments, it would be good if we would not do that (along with correcting the XUL for it) and instead port the nicer name that we have back to Thunderbird.

(In reply to alta88 from comment #28) > Created attachment 8407519 [details] [diff] [review] > suite.patch WFM on Trunk SM Linux x86_64

I am still Holding to SeaMonkey 25 Beta and resisting Moving to 26 Beta1 because of this Any "Cures" should be applied to Mac version of SM as well

So maybe I totally missed something while testing a few things, but I can simply fix this bug by adding <!ENTITY recentFolders.label "Recent"> <!ENTITY recentFolders.accesskey "R"> to suite/locales/en-US/chrome/mailnews/messenger.dtd as searchWidgets.xml replaces the msgFolderPickerOverlay.dtd reference with a messenger.dtd reference. Of course for trunk we could still replace the FilterListDialog.js/xul with the TB version, but for branches we could look into fixing it somewhere there. Maybe we could even restore the msgFolderPickerOverlay.dtd reference as msgFolderPickerOverlay.dtd still exists in mail/ on comm-beta/TB 29 (see http://hg.mozilla.org/comm-central/diff/2db24bbb4a11/mailnews/base/search/content/searchWidgets.xml for patch that changed this). That file is gone though in comm-aurora/TB 30, so need a different solution there.

This seems to fix the issue for SeaMonkey, currently testing if Thunderbird still works fine (by code inspection it should, but making sure it does).

Comment on attachment 8407873 [details] [diff] [review] comm-beta/SeaMonkey 2.26/TB 29 patch Review of attachment 8407873 [details] [diff] [review]: ----------------------------------------------------------------- good catch. the goal was to remove all msgFolderPickerOverlay.[js|xul|dtd] from /mail, and if you take the FilterListDialog.[js|xul] and strings there will be one sole remaining use in suite's searchDialog.[js|xul]. Bug 964425 removed the actual msgFolderPickerOverlay.dtd file, so for comm 30 all you need to do is back out that patch.

Well, the patch from Bug 964425 already is on aurora, I think string changes on aurora should be avoided.

It's not really a string change, it's restoring a file with an existing entity, no? Unless I'm mistaken, there should be no l10n work at all.

(In reply to alta88 from comment #36) > It's not really a string change, it's restoring a file with an existing > entity, no? Unless I'm mistaken, there should be no l10n work at all. Localizers probably have already removed the file from their localizations. Any way we do it, this will be a late string change in any case, but we should keep it as small as possible on anything string-frozen, which includes Aurora.

Comment on attachment 8407519 [details] [diff] [review] suite.patch Review of attachment 8407519 [details] [diff] [review]: ----------------------------------------------------------------- [FTR this is just a drive-by patch-based heads up, not an actual review. Didn't apply/test anything.] ::: suite/locales/en-US/chrome/mailnews/FilterListDialog.dtd @@ +33,5 @@ > <!ENTITY folderPickerPrefix.label "Run selected filter(s) on:"> > <!ENTITY folderPickerPrefix.accesskey "c"> > +<!ENTITY helpButton.label "Help"> > +<!ENTITY helpButton.accesskey "H"> > +<!ENTITY closeCmd.key "W"> Trailing whitespace on closeCmd.key line ::: suite/locales/en-US/chrome/mailnews/filter.properties @@ +62,5 @@ > filterAction3=deleted > filterAction4=marked as read > filterAction5=thread killed > filterAction6=thread watched > +filterAction7=starred I think this change is wrong. SM doesn't use "starred" anywhere else, we still refer to this state as "flagged".

[Approval Request Comment] Regression caused by (bug #): Bug 878805 User impact if declined: Broken message filter dialog in SeaMonkey Testing completed (on m-c, etc.): Works fine locally in SeaMonkey and Thunderbird Risk to taking this patch (and alternatives if risky): Almost no risk, adds back a reference to an already existing .dtd file String changes made by this patch: - (the .dtd file already exists on the comm-beta branch) Totally forgot to add back the entry to jar.mn. According to #l10n channel this should be ok as the tools don't use jar.mn to determinate what files to translate/delete. Also see http://mxr.mozilla.org/l10n-mozilla-beta/find?string=/mail/chrome/messenger/msgFolderPickerOverlay.dtd the file still seems to be there for all locales.

Somewhat ugly patch, but that's the only more or less clean solution I can think of right now. I tested it locally that it works with Thunderbird and SeaMonkey on aurora branch.

Comment on attachment 8408238 [details] [diff] [review] comm-aurora/SeaMonkey 2.27/TB 30 patch Review of attachment 8408238 [details] [diff] [review]: ----------------------------------------------------------------- it's just to get through the transition to comm 31 where hopefully things will realign.

Comment on attachment 8408071 [details] [diff] [review] comm-beta/SeaMonkey 2.26/TB 29 patch a=me

Comment on attachment 8408071 [details] [diff] [review] comm-beta/SeaMonkey 2.26/TB 29 patch Pushed: https://hg.mozilla.org/releases/comm-beta/rev/8b54c0720272

Comment on attachment 8408238 [details] [diff] [review] comm-aurora/SeaMonkey 2.27/TB 30 patch [Approval Request Comment] Regression caused by (bug #): Bug 878805 User impact if declined: Broken message filter dialog in SeaMonkey Testing completed (on m-c, etc.): Works fine locally, branch-only patch Risk to taking this patch (and alternatives if risky): Very few risk, small patch String changes made by this patch: -

Comment on attachment 8407519 [details] [diff] [review] suite.patch I can confirm that this large patch fixes the issue on trunk, FWIW.

Comment on attachment 8407519 [details] [diff] [review] suite.patch This change is out of scope for this bug. (It isn't really in scope for bug 507676 either, since it includes a number of unnecessary regressions.)

(In reply to neil@parkwaycc.co.uk from comment #47) > Created attachment 8409326 [details] [diff] [review] > Possible patch These changes are enough to fix the problem for my SM Trunk Linux x86_64.

Just tried to use the new 26 Beta2 version and corrupted my SeaMonkey install So bad I had to go to FireFox (pre-Australis version) and Down load Beta 25 again. From Mozilla Using Mac OSX.9.2 when clicked on was message (after restart of SeaMonkey) Application Defective ether corrupt or components missing. Went back to Beta25 version. Think I am going to stick until go to Beta 27 give you time to get your acts together.

Comment on attachment 8408238 [details] [diff] [review] comm-aurora/SeaMonkey 2.27/TB 30 patch a=me for comm-aurora

Comment on attachment 8409326 [details] [diff] [review] Possible patch Stealing review from mcsmurf. This patch works for me r+

Pushed comm-central changeset 1485af0790aa.

So was the idea of replacing the files with TB's versions abandoned?

Pushed to aurora: https://hg.mozilla.org/releases/comm-aurora/rev/1290418ea5f4

(In reply to :aceman from comment #55) > So was the idea of replacing the files with TB's versions abandoned? I'd say only as far as this bug is concerned, which has the focus on fixing a regression. For that, with branches in mind, the safest approach is best. Beyond that, probably in another bug, your approach might be perfectly fine (as in take your patch as-is).

confirmed: the last patch and release of 2.27a2 is fixed!