Closed
Bug 422474
Opened 17 years ago
Closed 17 years ago
Excise nsMsgFilterDataSource and friends
Categories
(Thunderbird :: General, defect)
Thunderbird
General
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 3
People
(Reporter: jminta, Assigned: jminta)
References
Details
Attachments
(1 file, 3 obsolete files)
(deleted),
patch
|
Bienvenu
:
review+
Bienvenu
:
superreview+
|
Details | Diff | 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.
Assignee | ||
Updated•17 years ago
|
Attachment #308918 -
Attachment description: scapel → scalpel
Updated•17 years ago
|
OS: Mac OS X → All
Hardware: PC → All
Assignee | ||
Comment 1•17 years ago
|
||
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)
Comment 2•17 years ago
|
||
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
Comment 3•17 years ago
|
||
(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.
Comment 4•17 years ago
|
||
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 5•17 years ago
|
||
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)
Assignee | ||
Comment 6•17 years ago
|
||
Unbitrotted patch.
Attachment #309020 -
Attachment is obsolete: true
Attachment #322527 -
Flags: superreview?(bienvenu)
Attachment #322527 -
Flags: review?(bienvenu)
Comment 7•17 years ago
|
||
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-
Assignee | ||
Comment 8•17 years ago
|
||
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 9•17 years ago
|
||
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+
Assignee | ||
Comment 10•17 years ago
|
||
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
Updated•16 years ago
|
Target Milestone: --- → Thunderbird 3
Updated•16 years ago
|
Comment 11•16 years ago
|
||
(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.
Comment 12•15 years ago
|
||
(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.
Description
•