Open Bug 507601 Opened 10 years ago Updated 26 days ago

Port |Bug 414038 - Replace rdf-driven folder pane with a js-driven/non-rdf treeview| to SeaMonkey

Categories

(SeaMonkey :: MailNews: Message Display, defect, minor)

defect
Not set
minor

Tracking

(seamonkey2.1 wontfix)

Tracking Status
seamonkey2.1 --- wontfix

People

(Reporter: sgautherie, Unassigned)

References

(Depends on 4 open bugs, Blocks 6 open bugs)

Details

(Whiteboard: [Wait for bug 510793])

Attachments

(2 files, 2 obsolete files)

No description provided.
Flags: wanted-seamonkey2?
Getting us to same same API as Thunderbird 3 is surely something we want, but we probably should only take it for 2.0 if it has patch and reviews for the last beta, as it might need some shaking out (even though we hopefully can avoid the regressions Thunderbird saw).
Flags: wanted-seamonkey2? → wanted-seamonkey2+
Depends on: 507421
Assignee: nobody → sgautherie.bz
Keywords: helpwanted
Depends on: 510646
Depends on: 439839
This port also removes the global var RDF usage in GetFolderAttribute() ;-)

To be applied on top of bug 460953 patch D!
Attachment #394656 - Flags: review?(iann_bugzilla)
Depends on: 510685
Depends on: 510716
Av1, with an added line that Thunderbird (already) had.
Attachment #394656 - Attachment is obsolete: true
Attachment #394679 - Flags: review?(iann_bugzilla)
Attachment #394656 - Flags: review?(iann_bugzilla)
Attached patch (Bv1) Clean up folder selection (obsolete) — Splinter Review
To be applied on top of bug 510716 patch!
Attachment #394685 - Flags: review?(iann_bugzilla)
Comment on attachment 394685 [details] [diff] [review]
(Bv1) Clean up folder selection

>--- a/suite/mailnews/mail3PaneWindowCommands.js
>+++ b/suite/mailnews/mail3PaneWindowCommands.js
>-      var folderTree = GetFolderTree();
>-      var startIndex = {};
>-      var endIndex = {};
>-      folderTree.view.selection.getRangeAt(0, startIndex, endIndex);
>-      if (startIndex.value >= 0) {
>+        let folders = GetSelectedMsgFolders();
>+
>+        if (folders.length) {
>         var canDeleteThisFolder;

Nit: you want your new lines on the same indentation as the old ones, I guess, not one level deeper :)
(In reply to comment #5)
> Nit: you want your new lines on the same indentation as the old ones, I guess,
> not one level deeper :)

Wrong guess ;-> The indentation in this file is a mess(!), but
the new indentation is the correct one. (see next line)
Status: NEW → ASSIGNED
(In reply to comment #6)
> (In reply to comment #5)
> > Nit: you want your new lines on the same indentation as the old ones, I guess,
> > not one level deeper :)
> 
> Wrong guess ;-> The indentation in this file is a mess(!), but
> the new indentation is the correct one. (see next line)

No. In your patch, the let and if are indented against things on the same level, and the var inside the if is on the same indentation level as the if. That's just wrong.
Bv1, with comment 7 suggestion(s).

(In reply to comment #7)
> the var inside the if is on the same indentation level as the if.
> That's just wrong.

Ah: fixing the indentation of the next var too, that I agree with ;-)
Attachment #394685 - Attachment is obsolete: true
Attachment #394726 - Flags: review?(iann_bugzilla)
Attachment #394685 - Flags: review?(iann_bugzilla)
Depends on: 510793
Depends on: 510731
Attachment #394679 - Flags: review?(iann_bugzilla) → review+
Comment on attachment 394726 [details] [diff] [review]
(Bv1a) Clean up folder selection
[Checkin: Comment 18]

I'll remove once you unbitrot it from landing Av1a
Attachment #394726 - Flags: review?(iann_bugzilla)
(In reply to comment #9)
> (From update of attachment 394726 [details] [diff] [review])
> I'll remove once you unbitrot it from landing Av1a

*review
Attachment #394726 - Flags: review?(iann_bugzilla)
Comment on attachment 394726 [details] [diff] [review]
(Bv1a) Clean up folder selection
[Checkin: Comment 18]

(In reply to comment #9)
> I'll review once you unbitrot it from landing Av1a

Actually, Bv1a applies cleanly on top of Av1a.
(which I'll land asaic.)
Comment on attachment 394679 [details] [diff] [review]
(Av1a) Remove GetFolderAttribute() + Bug 439839
[Checkin: Comment 12+16]


http://hg.mozilla.org/comm-central/rev/d6e41eeedfa8
Attachment #394679 - Attachment description: (Av1a) Remove GetFolderAttribute() + Bug 439839 → (Av1a) Remove GetFolderAttribute() + Bug 439839 [Checkin: Comment 12]
Attachment #394726 - Flags: review?(iann_bugzilla) → review+
Comment on attachment 394679 [details] [diff] [review]
(Av1a) Remove GetFolderAttribute() + Bug 439839
[Checkin: Comment 12+16]

https://bugzilla.mozilla.org/attachment.cgi?id=394679&action=diff#a/suite/mailnews/mailContextMenus.js_sec3
Looks like canCompact got removed but is still used? (kill-rdf is hard! :-))
(In reply to comment #13)
> (From update of attachment 394679 [details] [diff] [review])
> https://bugzilla.mozilla.org/attachment.cgi?id=394679&action=diff#a/suite/mailnews/mailContextMenus.js_sec3
> Looks like canCompact got removed but is still used? (kill-rdf is hard! :-))

A simple
let canCompact = msgFolder.canCompact;
should suffice?
(In reply to comment #14)
> (In reply to comment #13)
> > (From update of attachment 394679 [details] [diff] [review] [details])
> > https://bugzilla.mozilla.org/attachment.cgi?id=394679&action=diff#a/suite/mailnews/mailContextMenus.js_sec3
> > Looks like canCompact got removed but is still used? (kill-rdf is hard! :-))
> 
> A simple
> let canCompact = msgFolder.canCompact;
> should suffice?
*cough* oops, folder rather than msgFolder
(In reply to comment #13)
> Looks like canCompact got removed but is still used? (kill-rdf is hard! :-))

Ah, good catch!
TB don't have this if block, I must have missed it or got distracted by the whitespace thing :-/

(In reply to comment #16)
> pushed as http://hg.mozilla.org/comm-central/rev/0adcab6df66c

Thanks.
Comment on attachment 394726 [details] [diff] [review]
(Bv1a) Clean up folder selection
[Checkin: Comment 18]


http://hg.mozilla.org/comm-central/rev/a1e2cdc96222
Attachment #394726 - Attachment description: (Bv1a) Clean up folder selection → (Bv1a) Clean up folder selection [Checkin: Comment 18]
Whiteboard: [Wait for bug 510793]
Attachment #394679 - Attachment description: (Av1a) Remove GetFolderAttribute() + Bug 439839 [Checkin: Comment 12] → (Av1a) Remove GetFolderAttribute() + Bug 439839 [Checkin: Comment 12+16]
Blocks: 513522
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+
Blocks: 456156
Flags: wanted-seamonkey2.1+
Blocks: 657604
Blocks: 657607
No longer blocks: 657604
No longer blocks: 460953
Blocks: 507669
St least mail news and the sidebar are now broken becoause of xul template removals and other stuff.
Assignee: bugzillamozillaorg_serge_20140323 → nobody
Status: ASSIGNED → NEW
Blocks: 86056
Blocks: 1584728
You need to log in before you can comment on or make changes to this bug.