Closed Bug 460953 Opened 11 years ago Closed 8 years ago

Port jminta's kill-rdf to SeaMonkey where applicable

Categories

(SeaMonkey :: MailNews: General, defect)

defect
Not set

Tracking

(seamonkey2.1 wontfix)

RESOLVED FIXED
seamonkey2.0b2
Tracking Status
seamonkey2.1 --- wontfix

People

(Reporter: kairo, Assigned: sgautherie)

References

(Blocks 1 open bug)

Details

(Keywords: meta)

Attachments

(6 files, 5 obsolete files)

8.20 KB, patch
iann_bugzilla
: review+
Bienvenu
: superreview+
Details | Diff | Splinter Review
3.74 KB, patch
iann_bugzilla
: review+
Details | Diff | Splinter Review
14.54 KB, patch
iann_bugzilla
: review+
Details | Diff | Splinter Review
5.01 KB, patch
iann_bugzilla
: review+
Details | Diff | Splinter Review
9.08 KB, patch
iann_bugzilla
: review+
Details | Diff | Splinter Review
7.90 KB, patch
iann_bugzilla
: review+
Details | Diff | Splinter Review
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.
Flags: wanted-seamonkey2+
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).
Depends on: 421382
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.
Assignee: nobody → sgautherie.bz
Status: NEW → ASSIGNED
Attachment #391327 - Flags: superreview?(bienvenu)
Attachment #391327 - Flags: review?(iann_bugzilla)
Attachment #391351 - Flags: review?(iann_bugzilla)
Depends on: 439364
Depends on: 439373
Porting bug 437869 may not be possible yet, so I'll wait before attaching the patch.
Depends on: 437869
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!
(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 ;-)
Depends on: 453820
+    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?
Attachment #391585 - Flags: review?(iann_bugzilla)
Depends on: 436357
Keywords: helpwanted
Depends on: 444913
Bv1, without bug 506004 (checked in) fix.
Attachment #391351 - Attachment is obsolete: true
Attachment #391699 - Flags: review?(iann_bugzilla)
Attachment #391351 - Flags: review?(iann_bugzilla)
Depends on: 506004
Skipping |// Make msgFolder point to the real folder, [...]|,
which was removed in bug 439364.
Attachment #391828 - Flags: review?(iann_bugzilla)
Bv1a, updated to be applied after patch Ev1 and not conflict.
Attachment #391830 - Flags: review?(iann_bugzilla)
Attachment #391699 - Attachment is obsolete: true
Attachment #391699 - Flags: review?(iann_bugzilla)
Depends on: 438786, 436718
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.
Attachment #391851 - Flags: review?(iann_bugzilla)
Depends on: 439378
Depends on: 507601
Depends on: 507669
Depends on: 507676
Blocks: TB2SM
(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.
Here is were we are:
6+1 trivial patches,
2+1 separate major bugs,
1 tracking/future bug.
Mnyromyr, feel free to take over the reviews if Ian is not around.
Attachment #391327 - Flags: superreview?(bienvenu) → superreview+
Attachment #391327 - Flags: review?(iann_bugzilla) → review+
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.
Attachment #391830 - Flags: review?(iann_bugzilla) → review-
Attachment #391543 - Flags: review?(iann_bugzilla) → review+
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
Attachment #391830 - Flags: review- → review+
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.
Attachment #391585 - Flags: review?(iann_bugzilla) → review+
(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?
Attachment #391828 - Flags: review?(iann_bugzilla) → review+
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 on attachment 391327 [details] [diff] [review]
(Av1) Bug 421382 - Remove dordfcommand, getcompositedatasource
[Checkin: Comment 22]


http://hg.mozilla.org/comm-central/rev/f5b9755b2419
Attachment #391327 - Attachment description: (Av1) Bug 421382 - Remove dordfcommand, getcompositedatasource → (Av1) Bug 421382 - Remove dordfcommand, getcompositedatasource [Checkin: Comment 22]
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
Attachment #391543 - Attachment description: (Cv1) Bug 439373 - Remove front end users of nsIMsgRDFDatasource → (Cv1) Bug 439373 - Remove front end users of nsIMsgRDFDatasource [Checkin: Comment 23]
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).
Attachment #391828 - Attachment description: (Ev1) Bug 436718 (part 1) + bug 438786 → (Ev1) Bug 436718 (part 1) + bug 438786 [Checkin: Comment 24]
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
}
Attachment #391830 - Attachment description: (Bv1b) Bug 439364 - Eliminate GetResourceForUri → (Bv1b) Bug 439364 - Eliminate GetResourceForUri [Checkin: Comment 25]
Attachment #391851 - Flags: review?(iann_bugzilla) → review+
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
Attachment #391851 - Attachment description: (Fv1) Bug 436718 (part 2) + bug 439378 → (Fv1) Bug 436718 (part 2) + bug 439378 [Checkin: Comment 26]
Keywords: meta
Target Milestone: --- → seamonkey2.0b2
Attachment #391585 - Flags: review?(neil)
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 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;
Attachment #391585 - Flags: review?(neil)
Attached patch (Dv1a) Bug 453820 (obsolete) — Splinter Review
Dv1, with (fixed) comment 28 suggestion(s).
Attachment #391585 - Attachment is obsolete: true
Attachment #393954 - Flags: review?(iann_bugzilla)
Dv1a, but moving one line removal to bug 507601 patch A (which had already landed for TB).
Attachment #393954 - Attachment is obsolete: true
Attachment #394655 - Flags: review?(iann_bugzilla)
Attachment #393954 - Flags: review?(iann_bugzilla)
Attachment #394655 - Flags: review?(iann_bugzilla) → review+
Comment on attachment 394655 [details] [diff] [review]
(Dv1b) Bug 453820
[Checkin: Comment 31]


http://hg.mozilla.org/comm-central/rev/3fbc785c37a1
Attachment #394655 - Attachment description: (Dv1b) Bug 453820 → (Dv1b) Bug 453820 [Checkin: Comment 31]
Whiteboard: [wait for blockers then proceed with patch G]
Not wanted for 2.0 any more at this stage, let's push it to 2.1
Flags: wanted-seamonkey2.1+
Flags: wanted-seamonkey2.0-
Flags: wanted-seamonkey2.0+
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.
Whiteboard: [wait for blockers then proceed with patch G] → [wait for blockers then proceed with patch G] [patches A to F: fixed-seamonkey2.0b2]
Target Milestone: seamonkey2.0b2 → ---
Flags: wanted-seamonkey2.1+
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
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.0b2
Depends on: 654864
Blocks: 657607
No longer blocks: 657607
Bug 657607 is the new tracker.
Blocks: 657607
No longer blocks: TB2SM
No longer depends on: mail-killrdf, 444913, 507601, 507669, 507676, 654864
Whiteboard: [wait for blockers then proceed with patch G] [patches A to F: fixed-seamonkey2.0b2]
Attachment #391917 - Attachment description: (Gv1) Bug 437869 - Remove the RDF global object → (Gv1) Bug 437869 - Remove the RDF global object [Moved to bug 657604]
Attachment #391917 - Attachment is obsolete: true
No longer depends on: 437869
Blocks: 507669
You need to log in before you can comment on or make changes to this bug.