Closed Bug 422474 Opened 17 years ago Closed 17 years ago

Excise nsMsgFilterDataSource and friends

Categories

(Thunderbird :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3

People

(Reporter: jminta, Assigned: jminta)

References

Details

Attachments

(1 file, 3 obsolete files)

Attached patch scalpel (obsolete) (deleted) — Splinter Review
This is one of the simplest rdf datasources to remove, since it only has one real consumer, the filter window. This patch fixes the filter-dialog to generate its data without the datasource. We can remove RDF entirely from this dialog once the xbl-folders land (and that will shorten up the code even further). I haven't confirmed that the backend makefile changes work yet, since something went wrong with my tree that requires a world-rebuild, but the front-end stuff should be good to go. I'll request review once I confirm that and/or make the necessary changes. Note that I don't have a very clear understanding of delegates, so someone else may be able to identify places where the delegate factories are used that I haven't spotted.
Attachment #308918 - Attachment description: scapel → scalpel
OS: Mac OS X → All
Hardware: PC → All
Attached patch sharper scalpel (obsolete) (deleted) — Splinter Review
This one actually compiles, and we now have working filters without any of the associated rdf. :-)
Attachment #308918 - Attachment is obsolete: true
Attachment #309020 - Flags: superreview?(bienvenu)
Attachment #309020 - Flags: review?(bienvenu)
Adding SM's features/fixes to port from TB to blocking list so that SM devs know about this and can pick it up when they have time.
Blocks: TB2SM
(In reply to comment #2) >Adding SM's features/fixes to port from TB to blocking list so that SM devs >know about this and can pick it up when they have time. Or if anyone else wants to pick this up, I'd advise them to only concentrate on the RDF removal and not bother with the miscellaneous changes.
this patch didn't apply cleanly against the trunk - I've tried to apply it by hand but if you've got a newer trunk patch, that would be helpful...I'll try running with my hand-merged patch as well.
Comment on attachment 309020 [details] [diff] [review] sharper scalpel clearing request since this has bit-rotted...
Attachment #309020 - Flags: superreview?(bienvenu)
Attachment #309020 - Flags: review?(bienvenu)
Attached patch re-sharpened scalpel (obsolete) (deleted) — Splinter Review
Unbitrotted patch.
Attachment #309020 - Attachment is obsolete: true
Attachment #322527 - Flags: superreview?(bienvenu)
Attachment #322527 - Flags: review?(bienvenu)
Comment on attachment 322527 [details] [diff] [review] re-sharpened scalpel as discussed on irc, you'll need the qute changes. this should go away before checking in :-) -function doHelpButton() -{ - openHelp("mail-filters"); + var list = document.getElementById("fitlerList") + for each (var item in list.selectedItems) + toggleFilter(item, list.getIndexOfItem(item)); the checkbox in the enabled column is no longer centered, which looks a bit odd, on windows. the delete button doesn't work for me, console message: JavaScript error: chrome://messenger/content/FilterListDialog.js, line 273: gPro mptService is not defined When I select a filter and click the Move Up button, the filter moves up, but the selection is lost, so the button disables, which makes it painful to move a filter more than one row at a time.
Attachment #322527 - Flags: superreview?(bienvenu)
Attachment #322527 - Flags: superreview-
Attachment #322527 - Flags: review?(bienvenu)
Attachment #322527 - Flags: review-
Attached patch scalpel v3 (deleted) — Splinter Review
This fixes the delete and selection issues mentioned in comment #7. I've kept the doHelpButton removal, since that function only appears to be called by http://mxr.mozilla.org/seamonkey/source/toolkit/obsolete/content/dialogOverlay.xul#60 (and friends), which aren't applicable to the filter dialog. I didn't try to fix the centering issue in qute, since I'd really just be guessing there. If someone on windows wants to play around with mail/themes/qute/mail/filterDialog.css to get that right, i'd appreciate it.
Attachment #322527 - Attachment is obsolete: true
Attachment #322554 - Flags: superreview?(bienvenu)
Attachment #322554 - Flags: review?(bienvenu)
Comment on attachment 322554 [details] [diff] [review] scalpel v3 great, thanks. We should make sure the SM folks know about this so that they can make the corresponding changes and then remove the #ifndef MOZ_THUNDERBIRD parts of this patch.
Attachment #322554 - Flags: superreview?(bienvenu)
Attachment #322554 - Flags: superreview+
Attachment #322554 - Flags: review?(bienvenu)
Attachment #322554 - Flags: review+
Patch checked in. Regression testing involves all of the Message Filters dialog. Anything and everything in there is suspect. Checking in mail/base/content/FilterListDialog.js; /cvsroot/mozilla/mail/base/content/FilterListDialog.js,v <-- FilterListDialog.js new revision: 1.17; previous revision: 1.16 done Checking in mail/base/content/FilterListDialog.xul; /cvsroot/mozilla/mail/base/content/FilterListDialog.xul,v <-- FilterListDialog.xul new revision: 1.9; previous revision: 1.8 done Checking in mail/themes/pinstripe/mail/filterDialog.css; /cvsroot/mozilla/mail/themes/pinstripe/mail/filterDialog.css,v <-- filterDialog.css new revision: 1.6; previous revision: 1.5 done Checking in mail/themes/qute/mail/filterDialog.css; /cvsroot/mozilla/mail/themes/qute/mail/filterDialog.css,v <-- filterDialog.css new revision: 1.7; previous revision: 1.6 done Checking in mailnews/base/build/nsMsgFactory.cpp; /cvsroot/mozilla/mailnews/base/build/nsMsgFactory.cpp,v <-- nsMsgFactory.cpp new revision: 1.130; previous revision: 1.129 done Checking in mailnews/base/public/nsMsgBaseCID.h; /cvsroot/mozilla/mailnews/base/public/nsMsgBaseCID.h,v <-- nsMsgBaseCID.h new revision: 1.15; previous revision: 1.14 done Checking in mailnews/base/search/src/Makefile.in; /cvsroot/mozilla/mailnews/base/search/src/Makefile.in,v <-- Makefile.in new revision: 1.53; previous revision: 1.52 done Checking in mailnews/build/nsMailModule.cpp; /cvsroot/mozilla/mailnews/build/nsMailModule.cpp,v <-- nsMailModule.cpp new revision: 1.52; previous revision: 1.51 done
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3
Depends on: 437099
Depends on: 455802
Blocks: 460952
No longer blocks: TB2SM
(In reply to comment #9) > We should make sure the SM folks know about this Nobody actually told us; we only found out trying to fix bug 460952.
(In reply to comment #3) > Or if anyone else wants to pick this up, I'd advise them to only concentrate on > the RDF removal and not bother with the miscellaneous changes. I assume this is what was done in bug 436357.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: