Port jminta's kill-rdf to SeaMonkey where applicable

RESOLVED FIXED in seamonkey2.0b2

Status

RESOLVED FIXED
11 years ago
6 years ago

People

(Reporter: kairo, Assigned: sgautherie)

Tracking

(Blocks: 1 bug, {meta})

Trunk
seamonkey2.0b2
Dependency tree / graph
Bug Flags:
wanted-seamonkey2.0 -

SeaMonkey Tracking Flags

(seamonkey2.1 wontfix)

Details

Attachments

(6 attachments, 5 obsolete attachments)

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
(Reporter)

Description

11 years ago
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+
(Reporter)

Comment 1

10 years ago
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).
(Assignee)

Updated

10 years ago
Depends on: 421382
(Assignee)

Comment 3

10 years ago
Created attachment 391327 [details] [diff] [review]
(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.
Assignee: nobody → sgautherie.bz
Status: NEW → ASSIGNED
Attachment #391327 - Flags: superreview?(bienvenu)
Attachment #391327 - Flags: review?(iann_bugzilla)
(Assignee)

Comment 4

10 years ago
Created attachment 391351 [details] [diff] [review]
(Bv1) Bug 439364 - Eliminate GetResourceForUri
Attachment #391351 - Flags: review?(iann_bugzilla)
(Assignee)

Updated

10 years ago
Depends on: 439364
(Assignee)

Comment 5

10 years ago
Created attachment 391543 [details] [diff] [review]
(Cv1) Bug 439373 - Remove front end users of nsIMsgRDFDatasource
[Checkin: Comment 23]
Attachment #391543 - Flags: review?(iann_bugzilla)
(Assignee)

Updated

10 years ago
Depends on: 439373
(Assignee)

Comment 6

10 years ago
Porting bug 437869 may not be possible yet, so I'll wait before attaching the patch.
Depends on: 437869
(Reporter)

Comment 7

10 years ago
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!
(Assignee)

Comment 8

10 years ago
(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 ;-)
(Assignee)

Updated

10 years ago
Depends on: 453820
(Assignee)

Comment 9

10 years ago
Created attachment 391585 [details] [diff] [review]
(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?
Attachment #391585 - Flags: review?(iann_bugzilla)
(Assignee)

Updated

10 years ago
Depends on: 436357
(Assignee)

Updated

10 years ago
Keywords: helpwanted
(Assignee)

Updated

10 years ago
Depends on: 444913
(Assignee)

Comment 10

10 years ago
Created attachment 391699 [details] [diff] [review]
(Bv1a) Bug 439364 - Eliminate GetResourceForUri

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)
(Assignee)

Updated

10 years ago
Depends on: 506004
(Assignee)

Comment 11

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

Skipping |// Make msgFolder point to the real folder, [...]|,
which was removed in bug 439364.
Attachment #391828 - Flags: review?(iann_bugzilla)
(Assignee)

Comment 12

10 years ago
Created attachment 391830 [details] [diff] [review]
(Bv1b) Bug 439364 - Eliminate GetResourceForUri
[Checkin: Comment 25]

Bv1a, updated to be applied after patch Ev1 and not conflict.
Attachment #391830 - Flags: review?(iann_bugzilla)
(Assignee)

Updated

10 years ago
Attachment #391699 - Attachment is obsolete: true
Attachment #391699 - Flags: review?(iann_bugzilla)
(Assignee)

Updated

10 years ago
Depends on: 438786, 436718
(Assignee)

Comment 13

10 years ago
Created attachment 391851 [details] [diff] [review]
(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.
Attachment #391851 - Flags: review?(iann_bugzilla)
(Assignee)

Updated

10 years ago
Depends on: 439378
(Assignee)

Updated

10 years ago
Depends on: 507601
(Assignee)

Updated

10 years ago
Depends on: 507669
(Assignee)

Updated

10 years ago
Depends on: 507676
(Assignee)

Updated

10 years ago
Blocks: 360488
(Assignee)

Comment 14

10 years ago
Created attachment 391917 [details] [diff] [review]
(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.
(Assignee)

Comment 15

10 years ago
Here is were we are:
6+1 trivial patches,
2+1 separate major bugs,
1 tracking/future bug.
(Assignee)

Comment 16

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

Updated

10 years ago
Attachment #391327 - Flags: superreview?(bienvenu) → superreview+

Updated

10 years ago
Attachment #391327 - Flags: review?(iann_bugzilla) → review+

Comment 17

10 years ago
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-

Updated

10 years ago
Attachment #391543 - Flags: review?(iann_bugzilla) → review+

Comment 18

10 years ago
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 19

10 years ago
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+

Comment 20

10 years ago
(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?

Updated

10 years ago
Attachment #391828 - Flags: review?(iann_bugzilla) → review+

Comment 21

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

r=me with ChangeFolderByURI changed to ChangeFolder
(Assignee)

Comment 22

10 years ago
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]
(Assignee)

Comment 23

10 years ago
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]
(Assignee)

Comment 24

10 years ago
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]
(Assignee)

Comment 25

10 years ago
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]

Updated

10 years ago
Attachment #391851 - Flags: review?(iann_bugzilla) → review+
(Assignee)

Comment 26

10 years ago
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]
(Assignee)

Updated

10 years ago
Keywords: meta
Target Milestone: --- → seamonkey2.0b2
(Assignee)

Updated

10 years ago
Attachment #391585 - Flags: review?(neil)
(Assignee)

Comment 27

10 years ago
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)
(Assignee)

Comment 29

10 years ago
Created attachment 393954 [details] [diff] [review]
(Dv1a) Bug 453820

Dv1, with (fixed) comment 28 suggestion(s).
Attachment #391585 - Attachment is obsolete: true
Attachment #393954 - Flags: review?(iann_bugzilla)
(Assignee)

Comment 30

10 years ago
Created attachment 394655 [details] [diff] [review]
(Dv1b) Bug 453820
[Checkin: Comment 31]

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)

Updated

10 years ago
Attachment #394655 - Flags: review?(iann_bugzilla) → review+
(Assignee)

Comment 31

10 years ago
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]
(Assignee)

Updated

10 years ago
Whiteboard: [wait for blockers then proceed with patch G]
(Reporter)

Comment 33

10 years ago
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+
(Reporter)

Comment 34

9 years ago
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.
(Assignee)

Updated

9 years ago
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 → ---
(Reporter)

Updated

9 years ago
status-seamonkey2.1: --- → wanted
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
Last Resolved: 8 years ago
status-seamonkey2.1: wanted → wontfix
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.0b2
(Assignee)

Updated

8 years ago
Depends on: 654864
Blocks: 657607
No longer blocks: 657607
(Assignee)

Comment 36

8 years ago
Bug 657607 is the new tracker.
Blocks: 657607
No longer blocks: 360488
No longer depends on: 420506, 444913, 507601, 507669, 507676, 654864
Whiteboard: [wait for blockers then proceed with patch G] [patches A to F: fixed-seamonkey2.0b2]
(Assignee)

Updated

8 years ago
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
(Assignee)

Updated

8 years ago
No longer depends on: 437869
Blocks: 507669
You need to log in before you can comment on or make changes to this bug.