114989 - Expand / collapse thread ("+" and "-") icons are drawn incorrectly

Status: VERIFIED FIXED

(Core :: XUL, defect, P3)

Description

[Angus Davis]

As can be most clearly seen by the attachments I am about to make to this bug,
the expand/collapse thread icons are incorrect. I will immediately post a
follow-up comment to this bug the relevant attachments. Apologies if this is a
dup; I spent 10 minutes looking through the 1400+ open MailNews FE bugs and
couldn't find this one, although it should be obvious to anyone using MailNews.

Comment 1

[Angus Davis]

Attachment: Current expand/collapse thread UI in Mozilla

This image attachment shows the problem.

Comment 2

[Angus Davis]

Attachment: Correct, expected collapse/expand UI (taken from Communicator)

This image attachment shows the *expected* UI appearance for collapse/expand
icons.

Comment 3

[R.K.Aa.]

dup of bug 94813?

Comment 4

[eberry]

*** This bug has been marked as a duplicate of 94813 ***

Comment 5

[Jan Varga [:janv]]

Not quite dupe of 94813.
There is missing correct padding for twisty in classic theme.

Comment 6

[R.K.Aa.]

padding is incorrect in the screenshot in bug 94813 too..it seems the right
place to deal with it. Please add comment there.

Comment 7

[[e-mail] (gone - use [e-mail] instead)]

it's a dup, by the screenshot and seth's description

Comment 8

[R.K.Aa.]

fix for bug 94813 was just checked in.

Comment 9

[R.K.Aa.]

and the +/- twisties are still not centered. I see this under classic theme on
Linux as well. eopening. OS: All.

Comment 10

[scottputterman]

reassigning to [e-mail].  Taking a guess that this is probably an outliner
bug.

Comment 11

[Jan Varga [:janv]]

Attachment: current shot

I thought it was correct now, one pixel should do it

Comment 12

[Jan Varga [:janv]]

reminder:

don't line up with parent image, use only "indentation / 2"
or
line up twisty with parent image too

Comment 13

[R.K.Aa.]

i wonder if this just got fixed in bug 131097

Comment 14

[esther]

for 3-pane ui, QA to olga 

Comment 15

[Jan Varga [:janv]]

The patch that was checked in for bug 131097 looks good.
Anyway it can be enhanced a bit more.

1.
treechildren:-moz-tree-twisty {
  padding-top: 1px; 
}

this is just a workaround for a bug in the tree implemenatation of PaintTwisty()
We should not define this padding-top, since twisties are automatically centered
vertically (I found what's the bug, fixed in my tree).

2.
Greg, you have increased the indentation for win/linux classic
It would be more consistent to increase it for mac classic and modern too

Comment 16

[Greg Valure]

1. Feel free to remove it as part of a patch that would make it unnecessary. 
Just make sure the + and - always line up with the dotted lines, as they do in
native Windows apps.

2. I don't have or have access to a Mac, and wouldn't want to change settings
blindly for other platforms.  Looking through lxr I see that it doesn't even use
the same style of twisty.  If it has the same problem and can be fixed with the
same solution, make a similar patch.

Increasing the indentation on Modern isn't necessary.  Modern uses a different
style and size twisty, and has an appropriate amount of space between it and the
icon.  The only reason it was increased in Classic was because they were too
close under the new positioning.

Comment 17

[Jan Varga [:janv]]

Nominating for buffy.
This is just a cleanup.

Comment 18

[Samir Gehani]

Nav triage team: nsbeta1+/adt3

Comment 19

[Jan Varga [:janv]]

Attachment: fix

- correct centering of twisty and image
- don't line up vertical thread lines with parent images, just use
|mIndentation / 2|, this should be a little bit faster too

Comment 20

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 93305 [details] [diff] [review]
fix

sr=bzbarsky

Comment 21

[Brian Ryner (not reading)]

Comment on attachment 93305 [details] [diff] [review]
fix

Can you elaborate a bit on the change where you add half a pixel to the x/y
position?  It looks like the original intent was to align it on an even pixel;
with this change I'm not sure that will happen (where do fractional pixels go?)

Comment 22

[Jan Varga [:janv]]

Attachment: new patch

bryner, I removed substracting of 1 pixel completely as we discuss

Comment 23

[Brian Ryner (not reading)]

Comment on attachment 95426 [details] [diff] [review]
new patch

r/sr=bryner

Comment 24

[Jan Varga [:janv]]

fixed

Comment 25

[laurel]

OK using nov22 commercial trunk: win98, linux rh8