460953 - Port jminta's kill-rdf to SeaMonkey where applicable

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: General, defect)

Description

[Robert Kaiser]

Bug 420506 has a huge pile of work killing RDF in Thunderbird, we should port those parts where we have forked files for the same things to SeaMonkey as well.

Comment 1

[Robert Kaiser]

Frank, as I know you're looking into this, I removed a JS switch between the JS-driven and RDF-driven folder view stuff in the SeaMonkey copy of mailWidgets.xml, see my patch in bug 466795 (attachment 350145 [details] [diff] [review] - look for |var msgFolder;|, we need the "gFolderTreeView" branch of the removed if once we switch to the de-RDFed folderpane).

Comment 2

[Serge Gautherie (:sgautherie)]

Some useful mxr searches:

http://mxr.mozilla.org/comm-central/search?string=NC-rdf&case=on&find=%2FEditorSaveAsCharset%5C.xul%24
Found one matching line
http://mxr.mozilla.org/comm-central/search?string=nsIRDFService&case=on&find=%2FEditorSaveAsCharset%5C.js%24
Found one matching line

http://mxr.mozilla.org/comm-central/search?string=nsRDFCID&case=on&find=%2Fsuite%2F.*%5C.cpp%24
Found 3 matching lines in 3 files
http://mxr.mozilla.org/comm-central/search?string=NC-rdf&case=on&find=%2Fsuite%2F
Found 536 matching lines in 62 files
http://mxr.mozilla.org/comm-central/search?string=rdfservice&find=%2Fsuite%2F
Found 192 matching lines in 32 files

Comment 3

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) Bug 421382 - Remove dordfcommand, getcompositedatasource [Checkin: Comment 22]

Ftr,
*Most of bug 421382 has already been ported in bug 435290.
*SM did not had bug 422992 regression.
*Bug 436718 fixed both SM and TB.

Comment 4

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1) Bug 439364 - Eliminate GetResourceForUri

Comment 5

[Serge Gautherie (:sgautherie)]

Attachment: (Cv1) Bug 439373 - Remove front end users of nsIMsgRDFDatasource [Checkin: Comment 23]

Comment 6

[Serge Gautherie (:sgautherie)]

Porting bug 437869 may not be possible yet, so I'll wait before attaching the patch.

Comment 7

[Robert Kaiser]

I'd prefer to have separate bugs with one one patch per bug on the SeaMonkey side as well, if possible, then each one can be individually resolved and each one can track its own row of review comments.

That said, thanks a lot for picking this up!

Comment 8

[Serge Gautherie (:sgautherie)]

(In reply to comment #7)
> I'd prefer to have separate bugs with one one patch per bug on the SeaMonkey
> side as well, if possible, then each one can be individually resolved and each
> one can track its own row of review comments.

Well, I'm not expecting (m)any review comments on the patches I'm attaching here, as they are trivial ports only/mostly.
Nontheless, there will probably be a few major ports to do, and I will open separate bugs for those.

> That said, thanks a lot for picking this up!

Yeah, this seems to be the one SeaMonkey 2 wanted "feature" I can actually do ;-)

Comment 9

[Serge Gautherie (:sgautherie)]

Attachment: (Dv1) Bug 453820 - Audit front-end code for unnecessary rdf QIs

+    let srchFolderUri = currentLoadedFolder.msgDatabase.dBFolderInfo
+                                           .getCharProperty("searchFolderUri");

This line was checked in as-is by bug 11051.
Yet, can this (useless!?) variable/line be removed too?

Comment 10

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1a) Bug 439364 - Eliminate GetResourceForUri

Bv1, without bug 506004 (checked in) fix.

Comment 11

[Serge Gautherie (:sgautherie)]

Attachment: (Ev1) Bug 436718 (part 1) + bug 438786 [Checkin: Comment 24]

Skipping |// Make msgFolder point to the real folder, [...]|,
which was removed in bug 439364.

Comment 12

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1b) Bug 439364 - Eliminate GetResourceForUri [Checkin: Comment 25]

Bv1a, updated to be applied after patch Ev1 and not conflict.

Comment 13

[Serge Gautherie (:sgautherie)]

Attachment: (Fv1) Bug 436718 (part 2) + bug 439378 [Checkin: Comment 26]

I was conservative on mailWindowOverlay.* because I don't know if ._folder is already usable or if it needs folderPane.js (from bug 414038) or whatever.
Anyway, that additional change is unrelated to de-RDF.

Comment 14

[Serge Gautherie (:sgautherie)]

Attachment: (Gv1) Bug 437869 - Remove the RDF global object [Moved to bug 657604]

(In reply to comment #6)
> Porting bug 437869 may not be possible yet, so I'll wait before attaching the
> patch.

I'll request review after all the rest is done.

Comment 15

[Serge Gautherie (:sgautherie)]

Here is were we are:
6+1 trivial patches,
2+1 separate major bugs,
1 tracking/future bug.

Comment 16

[Serge Gautherie (:sgautherie)]

Mnyromyr, feel free to take over the reviews if Ian is not around.

Comment 17

[Ian Neal]

Comment on attachment 391830 [details] [diff] [review]
(Bv1b) Bug 439364 - Eliminate GetResourceForUri
[Checkin: Comment 25]

The first part (commandglue.js) of this patch seems to have been bitrotted.

Comment 18

[Ian Neal]

Comment on attachment 391830 [details] [diff] [review]
(Bv1b) Bug 439364 - Eliminate GetResourceForUri
[Checkin: Comment 25]

Ah I see, I was reviewing in alphabetic order. r=me

Comment 19

[Ian Neal]

Comment on attachment 391585 [details] [diff] [review]
(Dv1) Bug 453820 - Audit front-end code for unnecessary rdf QIs

>+    let srchFolderUri = currentLoadedFolder.msgDatabase.dBFolderInfo
>+                                           .getCharProperty("searchFolderUri");
>+    let re = new RegExp("^" + msgfolder.URI + "$|^" + msgfolder.URI + "\||\|" +
>+                          msgfolder.URI + "$|\|" + msgfolder.URI +"\|");
You have the indentation wrong here.

Comment 20

[Ian Neal]

(In reply to comment #9)
> Created an attachment (id=391585) [details]
> (Dv1) Bug 453820 - Audit front-end code for unnecessary rdf QIs
> 
> +    let srchFolderUri = currentLoadedFolder.msgDatabase.dBFolderInfo
> +                                          
> .getCharProperty("searchFolderUri");
> 
> This line was checked in as-is by bug 11051.
> Yet, can this (useless!?) variable/line be removed too?

As far as I can see it is not needed, have you tested to see if it breaks anything by removing it?

Comment 21

[Ian Neal]

Comment on attachment 391828 [details] [diff] [review]
(Ev1) Bug 436718 (part 1) + bug 438786
[Checkin: Comment 24]

r=me with ChangeFolderByURI changed to ChangeFolder

Comment 22

[Serge Gautherie (:sgautherie)]

Comment on attachment 391327 [details] [diff] [review]
(Av1) Bug 421382 - Remove dordfcommand, getcompositedatasource
[Checkin: Comment 22]


http://hg.mozilla.org/comm-central/rev/f5b9755b2419

Comment 23

[Serge Gautherie (:sgautherie)]

Comment on attachment 391543 [details] [diff] [review]
(Cv1) Bug 439373 - Remove front end users of nsIMsgRDFDatasource
[Checkin: Comment 23]


http://hg.mozilla.org/comm-central/rev/698c8a90292a

Comment 24

[Serge Gautherie (:sgautherie)]

Comment on attachment 391828 [details] [diff] [review]
(Ev1) Bug 436718 (part 1) + bug 438786
[Checkin: Comment 24]


http://hg.mozilla.org/comm-central/rev/13dbf303601e
Ev1, with s/var/let/g and comment 21 suggestion(s).

Comment 25

[Serge Gautherie (:sgautherie)]

Comment on attachment 391830 [details] [diff] [review]
(Bv1b) Bug 439364 - Eliminate GetResourceForUri
[Checkin: Comment 25]


http://hg.mozilla.org/comm-central/rev/950bbee408b3

after fixing context for
{
patching file suite/mailnews/commandglue.js
Hunk #3 FAILED at 1048
1 out of 3 hunks FAILED
}

Comment 26

[Serge Gautherie (:sgautherie)]

Comment on attachment 391851 [details] [diff] [review]
(Fv1) Bug 436718 (part 2) + bug 439378
[Checkin: Comment 26]


http://hg.mozilla.org/comm-central/rev/c5ec13021dbc

Comment 27

[Serge Gautherie (:sgautherie)]

Comment on attachment 391585 [details] [diff] [review]
(Dv1) Bug 453820 - Audit front-end code for unnecessary rdf QIs

>diff --git a/suite/mailnews/msgMail3PaneWindow.js b/suite/mailnews/msgMail3PaneWindow.js
>+    let re = new RegExp("^" + msgfolder.URI + "$|^" + msgfolder.URI + "\||\|" +
>+                          msgfolder.URI + "$|\|" + msgfolder.URI +"\|");

Neil, IanN asked me to double check this nit with you:
I like it like this, but Ian suggests to align the second line just after the '(';
which one do you prefer?

Comment 28

[[e-mail]]

Comment on attachment 391585 [details] [diff] [review]
(Dv1) Bug 453820 - Audit front-end code for unnecessary rdf QIs

>+    let srchFolderUri = currentLoadedFolder.msgDatabase.dBFolderInfo
>+                                           .getCharProperty("searchFolderUri");
>+    let re = new RegExp("^" + msgfolder.URI + "$|^" + msgfolder.URI + "\||\|" +
>+                          msgfolder.URI + "$|\|" + msgfolder.URI +"\|");
>+    return currentLoadedFolder.URI.match(re);
Neither actually; I think I'd prefer
return srchFolderUri.split("|").indexOf(currentLoadedFolder.URI) != -1;

Comment 29

[Serge Gautherie (:sgautherie)]

Attachment: (Dv1a) Bug 453820

Dv1, with (fixed) comment 28 suggestion(s).

Comment 30

[Serge Gautherie (:sgautherie)]

Attachment: (Dv1b) Bug 453820 [Checkin: Comment 31]

Dv1a, but moving one line removal to bug 507601 patch A (which had already landed for TB).

Comment 31

[Serge Gautherie (:sgautherie)]

Comment on attachment 394655 [details] [diff] [review]
(Dv1b) Bug 453820
[Checkin: Comment 31]


http://hg.mozilla.org/comm-central/rev/3fbc785c37a1

Comment 32

[Serge Gautherie (:sgautherie)]

Comment on attachment 391917 [details] [diff] [review]
(Gv1) Bug 437869 - Remove the RDF global object
[Moved to bug 657604]


This waits for blocking bugs.

See also:
http://mxr.mozilla.org/comm-central/search?string=%5B%5Ea-z%5C.%5DRDF%5C.&regexp=on&case=on&find=%2Fsuite%2F

Comment 33

[Robert Kaiser]

Not wanted for 2.0 any more at this stage, let's push it to 2.1

Comment 34

[Robert Kaiser]

This bug is open but targeted for seamonkey2.0b2, which has been released a significant time ago. Please set the target milestone to an appropriate value, "---" if it has no specific target.

Comment 35

[Justin Wood (:Callek)]

Ok, lots of patches here, last one landed for 2.0b2, resolving...

I know there is more "kill-RDF" work, but it won't happen in this bug, we need either a new _tracker_ or new bugs

Comment 36

[Serge Gautherie (:sgautherie)]

Bug 657607 is the new tracker.