422474 - Excise nsMsgFilterDataSource and friends

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Joey Minta]

Attachment: scalpel

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.

Comment 1

[Joey Minta]

Attachment: sharper scalpel

This one actually compiles, and we now have working filters without any of the associated rdf. :-)

Comment 2

[Mark Banner (:standard8)]

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.

Comment 3

[[e-mail]]

(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

[David :Bienvenu]

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

[David :Bienvenu]

Comment on attachment 309020 [details] [diff] [review]
sharper scalpel

clearing request since this has bit-rotted...

Comment 6

[Joey Minta]

Attachment: re-sharpened scalpel

Unbitrotted patch.

Comment 7

[David :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.

Comment 8

[Joey Minta]

Attachment: scalpel v3

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.

Comment 9

[David :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.

Comment 10

[Joey Minta]

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

Comment 11

[[e-mail]]

(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

[Serge Gautherie (:sgautherie)]

(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.