438793 - [Classic] Missing Treelines (in MailNews, Venkman, Inspector, etc.)

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Karsten Düsterloh]

This is basically a copy of bug 430852, because the issue is NOT fixed by adding random attributes to toolkit... :-/

With bug 425131 "fixed", both Windows and Linux are missing treelines in trees
in Classic. This is (a) violating the respective OS settings and (b) a real
PITA when reading threaded mailing lists or usenet.

Also affected are DOMI, Venkman and long hierarchical bookmark lists, and there's the chance that their owners don't agree. We need a list of our  hierarchical trees! (In some places, we use "flat trees" as a listbox substitution, which don't need treelines, of course - but I've no idea how many of those ~120 tree are flat...)

Attachment 318255 [details] [diff] in bug 430852 contains a working solution demo for all SeaMonkey trees in general, without touching every possibly interesting <tree> element, but it needs to be edited for Mac.

The cleanest way still to deal with this situation IMO is fixing bug 297438, so I'll investigate that first.

Setting onto SM2 radar.

Comment 1

[Robert Kaiser]

(In reply to comment #0)
> With bug 425131 "fixed", both Windows and Linux are missing treelines in trees
> in Classic. This is (a) violating the respective OS settings and (b) a real
> PITA when reading threaded mailing lists or usenet.

The a) here is wrong, it does honor OS settings, not violate them, as discussed in the other bug (the only thing it really violates is KDE, integration with which we don't support currently).

I'd object to any solution that wrongly re-introduces treelines on all our trees, because of the OS style violation it would be.

Comment 2

[Karsten Düsterloh]

(In reply to comment #1)
> (In reply to comment #0)
> > With bug 425131 "fixed", both Windows and Linux are missing treelines in
> > trees in Classic. This is (a) violating the respective OS settings and
> > (b) a real PITA when reading threaded mailing lists or usenet.
> 
> The a) here is wrong,

True, I plead in "cut and paste error". ;-) 

> it does honor OS settings,

Wrong.

> not violate them, as discussed in the other bug

As stated in the other bug, there just is *no* OS default for that in Windows.

Let's get a list first of all our >100 trees and see which ones are hierarchical (enough) in nature and thus may need treelines, before getting all heated. ;-)

Comment 3

[Joe Sabash [:JoeS1]]

IMO this is a major regression

Submitted bug 448456 for the thunderbird parts.

Comment 5

[Serge Gautherie (:sgautherie)]

(In reply to comment #4)
> *** Bug 449313 has been marked as a duplicate of this bug. ***

Is restoring the lines wanted only for Classic theme ?
From bug 449313 images, Modern seemed to have the lines too;
or maybe Modern did want to remove the lines ?

Comment 6

[Karsten Düsterloh]

Attachment: treelines for Venkman, v1 [checked in]

Let's start with Venkman. Venkman has seven trees, but only six of them are ever visible (views-overlay-target is hidden and probably superfluous). Of the six trees used, two () are just pseudo listboxes without a hierarchy. This patch fixes the remaining four...

Comment 7

[Alex Vincent [:WeirdAl]]

Patch looks simple enough for a rubberstamp, but I've never seen the treelines attribute before.  I see it mentioned in the list of bugs this depends on...  expect review tonight or tomorrow at the latest.

Comment 8

[Alex Vincent [:WeirdAl]]

Comment on attachment 335443 [details] [diff] [review]
treelines for Venkman, v1 [checked in]

I don't suppose there's a reftest for treelines anywhere, is there?

Comment 9

[Karsten Düsterloh]

(In reply to comment #8)
> I don't suppose there's a reftest for treelines anywhere, is there?

I don't think so and I'm not sure it would work anyway, but I'll have a look.

Comment 10

[Karsten Düsterloh]

Comment on attachment 335443 [details] [diff] [review]
treelines for Venkman, v1 [checked in]

Landed on CVS trunk aka 1.9.0.

Comment 11

[Karsten Düsterloh]

Attachment: treelines for DOMI [checked in]

DOMI has 18 trees, but only 6 of them are hierarchical:
- left pane:
  - accessible tree
  - DOM nodes
  - stylesheets
- right pane:
  - accessible object
  - JS object
  - binding content
This patch enables treelines for these trees.
(Mind bug 434080 if you're on Linux/GTK.)

Comment 12

[Shawn Wilsher :sdwilsh]

Comment on attachment 335965 [details] [diff] [review]
treelines for DOMI [checked in]

r=sdwilsh

Comment 13

[Karsten Düsterloh]

Comment on attachment 335965 [details] [diff] [review]
treelines for DOMI [checked in]

Landed on DOMI trunk, <http://hg.mozilla.org/dom-inspector/rev/62b317155d82>.

Comment 14

[Karsten Düsterloh]

The addressbook has five trees, only one being hierarchical. But this dirTree in addressbook.xul only has one sublevel at most:

addressbook1
+- mailinglist1
+- mailinglist2
+- mailinglist3
addressbook2
+- mailinglist4
addressbook3
+- mailinglist5

No need for treelines here due to lack of complexity, imo.

Comment 15

[Karsten Düsterloh]

Attachment: treelines for MailNews, v1

9/15 trees are hierarchical with more than one sublevel, thus patching. 
I specifically didn't touch the account manager, because all its branches are leaves. ;-)

David, do you want me to patch the respective /mail files as well?

Comment 16

[David :Bienvenu]

Comment on attachment 337985 [details] [diff] [review]
treelines for MailNews, v1

Karsten, it would be great if you could do the mail ones as well, thx!

Comment 17

[[e-mail]]

Comment on attachment 337985 [details] [diff] [review]
treelines for MailNews, v1

>+                    <tree id="searchTree"
>+                          treelines="true"
>+                          flex="1" 
>+                          disableKeyNavigation="true"
>+                          onkeypress="onSearchTreeKeyPress(event);"
>+                          onclick="SearchOnClick(event);"> 
I think this is the newsgroup "search results" flat view...

Comment 18

[[e-mail]]

Comment on attachment 337985 [details] [diff] [review]
treelines for MailNews, v1

>+      <tree class="foldersTree"
>+            treelines="true"
These lines are actually explicitly disabled in chrome://messenger/content/messenger.css (last but one rule).

Comment 19

[Karsten Düsterloh]

Attachment: treelines for /mailnews and /mail, v2 [checked in]

(In reply to comment #16)
> Karsten, it would be great if you could do the mail ones as well, thx!

/mail is now included:
14 trees, 6 of those hierarchical, only 4 with more than one sublevel.

(In reply to comment #17)
> I think this is the newsgroup "search results" flat view...

Right. Fixed.

(In reply to comment #18)
> >+      <tree class="foldersTree"
> >+            treelines="true"
> These lines are actually explicitly disabled in
> chrome://messenger/content/messenger.css (last but one rule).

But they should adhere to the treelines attribute and theming. Fixed.

Comment 20

[David :Bienvenu]

Comment on attachment 338199 [details] [diff] [review]
treelines for /mailnews and /mail, v2 [checked in]

thx, Karsten

Comment 21

[Karsten Düsterloh]

Comment on attachment 338199 [details] [diff] [review]
treelines for /mailnews and /mail, v2 [checked in]

Landed on trunk as <http://hg.mozilla.org/comm-central/rev/4ad411cb6f20>.

Comment 22

[Karsten Düsterloh]

Attachment: treelines for certificate details tree, v1 [checked in]

This patch adds treelines to the deeply nested certificate details tree (if allowed by the theme). This tree is found eg. in SeaMonkey under Preferences->Certificates->Manage Certificates->Servers->$Server-Certificate->View->Details.

Comment 23

[Karsten Düsterloh]

Attachment: treelines for help toc, v1 [checked in]

Activate treelines for the nested table of contents in the help viewer, so theme permits. (Is SM the only consumer left, actually?)

Comment 24

[Karsten Düsterloh]

Attachment: treelines for bookmarks, history and XUL directory viewer, v1 [checked in]

And as the final volume in this series: bookmarks (sidebar, manager and addition dialog), history (sidebar and manager) and XUL directory listing.

I specifically did not look closer upon these five calendar trees, since calendar doesn't build with SM as is currently:
  calendar/base/content/calendar-calendars-list.xul:  
    <tree id="calendar-list-tree-widget"
  calendar/base/content/calendar-task-tree.xml:      
    <xul:tree anonid="calendar-task-tree"
  calendar/base/content/calendar-unifinder.xul:     
    <tree id="unifinder-search-results-tree" flex="1"
  calendar/lightning/content/messenger-overlay-sidebar.xul:
    <tree id="calendar-list-tree-widget"  class="task-tree-subpane"
  calendar/sunbird/base/content/calendar.xul:
    <tree id="calendar-list-tree-widget" flex="1"/>

This can be done in a separate bug, if needed.

Comment 25

[Robert Kaiser]

not blocking, but will probably approve further patches for this as time until actually cutting the alpha allows.

Comment 26

[Karsten Düsterloh]

Comment on attachment 338963 [details] [diff] [review]
treelines for help toc, v1 [checked in]

Landed this on mozilla-central trunk as <http://hg.mozilla.org/mozilla-central/rev/75626c45e455>.

Comment 27

[Nelson Bolyard (seldom reads bugmail)]

Here's a note of thanks for restoring tree lines to the email folder and
message header panes.  I appreciate it!

Comment 28

[Karsten Düsterloh]

Comment on attachment 338968 [details] [diff] [review]
treelines for bookmarks, history and XUL directory viewer, v1 [checked in]

Landed on trunk as <http://hg.mozilla.org/comm-central/rev/2badeaeb4bdf>.

Comment 29

[doctor__j]

Just noticed that this treeline bug is also affecting the Preference pane.
The treelines are still missing in the trunk builds after checking in the fixes.

Comment 30

[Karsten Düsterloh]

> Just noticed that this treeline bug is also affecting the Preference pane.

I specifically only patched trees at least two sublevels deep. Trees used as listbox surrogates (= 0 levels) or just one level (eg. the preferences tree) I deemed not complex enough to require treelines.

Comment 31

[Kai Engert (:KaiE:)]

Comment on attachment 338962 [details] [diff] [review]
treelines for certificate details tree, v1 [checked in]

r=kaie

Comment 32

[Karsten Düsterloh]

Comment on attachment 338962 [details] [diff] [review]
treelines for certificate details tree, v1 [checked in]

Should be too late for a1, hence requesting a2 as well.

Comment 33

[Robert Kaiser]

Comment on attachment 338962 [details] [diff] [review]
treelines for certificate details tree, v1 [checked in]

a1 has been cut already, a2 doesn't need approval yet :)

Comment 34

[Karsten Düsterloh]

Comment on attachment 338962 [details] [diff] [review]
treelines for certificate details tree, v1 [checked in]

Landed this on mozilla-central trunk as <http://hg.mozilla.org/mozilla-central/rev/c6797cdbac48>.

Comment 35

[Karsten Düsterloh]

So, if I forgot any non-hierarchical trees with more than one sublevel, please file new bugs!