Closed Bug 438793 Opened 12 years ago Closed 12 years ago

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

Categories

(SeaMonkey :: UI Design, defect, critical)

defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED
seamonkey2.0a2

People

(Reporter: mnyromyr, Assigned: mnyromyr)

References

(Depends on 1 open bug)

Details

(Keywords: regression)

Attachments

(6 files, 1 obsolete file)

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.
Flags: blocking-seamonkey2.0a1?
(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.
(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. ;-)
IMO this is a major regression

Submitted bug 448456 for the thunderbird parts.
Component: XP Apps: GUI Features → UI Design
Duplicate of this bug: 449313
(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 ?
Blocks: 448456
Depends on: 425131
Summary: [Classic] Missing Treelines in MailNews → [Classic] Missing Treelines (in MailNews, Venkman, Inspector, etc.)
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...
Attachment #335443 - Flags: review?(ajvincent)
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 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?
Attachment #335443 - Flags: review?(ajvincent) → review+
(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 on attachment 335443 [details] [diff] [review]
treelines for Venkman, v1 [checked in]

Landed on CVS trunk aka 1.9.0.
Attachment #335443 - Attachment description: treelines for Venkman, v1 → treelines for Venkman, v1 [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.)
Attachment #335965 - Flags: superreview?
Attachment #335965 - Flags: review?(sdwilsh)
Attachment #335965 - Flags: superreview? → superreview?(neil)
Attachment #335965 - Flags: review?(sdwilsh) → review+
Comment on attachment 335965 [details] [diff] [review]
treelines for DOMI [checked in]

r=sdwilsh
Attachment #335965 - Flags: superreview?(neil) → superreview+
Comment on attachment 335965 [details] [diff] [review]
treelines for DOMI [checked in]

Landed on DOMI trunk, <http://hg.mozilla.org/dom-inspector/rev/62b317155d82>.
Attachment #335965 - Attachment description: treelines for DOMI → treelines for DOMI [checked in]
Blocks: 420105
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.
Attached patch treelines for MailNews, v1 (obsolete) — Splinter Review
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?
Attachment #337985 - Flags: superreview?
Attachment #337985 - Flags: review?(neil)
Attachment #337985 - Flags: superreview? → superreview?(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!
Attachment #337985 - Flags: superreview?(bienvenu) → superreview+
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...
Attachment #337985 - Flags: review?(neil) → review-
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).
(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.
Attachment #337985 - Attachment is obsolete: true
Attachment #338199 - Flags: superreview?(bienvenu)
Attachment #338199 - Flags: review?(neil)
Attachment #338199 - Flags: review?(neil) → review+
Attachment #338199 - Flags: superreview?(bienvenu) → superreview+
Comment on attachment 338199 [details] [diff] [review]
treelines for /mailnews and /mail, v2 [checked in]

thx, Karsten
Attachment #338199 - Flags: approval-seamonkey2.0a1?
Attachment #338199 - Flags: approval-seamonkey2.0a1? → approval-seamonkey2.0a1+
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>.
Attachment #338199 - Attachment description: treelines for /mailnews and /mail, v2 → treelines for /mailnews and /mail, v2 [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.
Attachment #338962 - Flags: review?
Attachment #338962 - Flags: review? → review?(kaie)
Activate treelines for the nested table of contents in the help viewer, so theme permits. (Is SM the only consumer left, actually?)
Attachment #338963 - Flags: review?(dao)
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.
Attachment #338968 - Flags: superreview?(neil)
Attachment #338968 - Flags: review?(iann_bugzilla)
Attachment #338963 - Flags: review?(dao) → review+
Attachment #338968 - Flags: superreview?(neil) → superreview+
not blocking, but will probably approve further patches for this as time until actually cutting the alpha allows.
Flags: blocking-seamonkey2.0a1? → blocking-seamonkey2.0a1-
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>.
Attachment #338963 - Attachment description: treelines for help toc, v1 → treelines for help toc, v1 [checked in]
Here's a note of thanks for restoring tree lines to the email folder and
message header panes.  I appreciate it!
Attachment #338968 - Flags: review?(iann_bugzilla) → review+
Attachment #338968 - Flags: approval-seamonkey2.0a1+
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>.
Attachment #338968 - Attachment description: treelines for bookmarks, history and XUL directory viewer, v1 → treelines for bookmarks, history and XUL directory viewer, v1 [checked in]
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.
> 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 on attachment 338962 [details] [diff] [review]
treelines for certificate details tree, v1 [checked in]

r=kaie
Attachment #338962 - Flags: review?(kaie) → review+
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.
Attachment #338962 - Flags: approval-seamonkey2.0a2?
Attachment #338962 - Flags: approval-seamonkey2.0a1?
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 :)
Attachment #338962 - Flags: approval-seamonkey2.0a2?
Attachment #338962 - Flags: approval-seamonkey2.0a1?
Attachment #338962 - Flags: approval-seamonkey2.0a1-
Attachment #338962 - Attachment description: treelines for certificate details tree, v1 → treelines for certificate details tree, v1 [checked in]
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>.
So, if I forgot any non-hierarchical trees with more than one sublevel, please file new bugs!
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: seamonkey2.0 → seamonkey2.0a2
Blocks: 620531
You need to log in before you can comment on or make changes to this bug.