652007 - Painting artifacts with RTL trees

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[[e-mail]]

Bug 649840 uncovered a painting bug with RTL trees.
In researching this bug, I have discovered the following:
*GetCoordsForCellItem doesn't take column alignment into account at all
*If the text is overflowing, AdjustForCellText doesn't take column alignment into account, so for instance if all the cells in RTL or right-aligned column overflow then they will have a ragged right margin
*GetItemWithinCellAt and PaintText force the text to be right-aligned without any margin or padding when RTL is in effect (possibly to work around the above bug) although the border and background paints in the expected position allowing for column alignment.
*PaintText also miscalculates the width of the text when it has borders or padding so PaintRow makes PaintSeparator paints over the border or padding.
*PaintRow also gets the left separator wrong; it tries to use the indentation instead of the twisty rect to make a top-level separator span the tree.

Comment 1

[[e-mail]]

Attachment: Proposed patch

Comment 2

[[e-mail]]

Comment on attachment 527681 [details] [diff] [review]
Proposed patch

>@@ -1275,25 +1275,17 @@ nsTreeBodyFrame::GetCoordsForCellItem
*GetCoordsForCellItem doesn't take column alignment into account at all

>@@ -1464,32 +1456,32 @@ nsTreeBodyFrame::AdjustForCellText
*AdjustForCellText doesn't take column alignment into account

>@@ -1623,18 +1615,16 @@ nsTreeBodyFrame::GetItemWithinCellAt
*GetItemWithinCellAt forces the text to be right-aligned when RTL is in effect

>@@ -3578,30 +3568,26 @@ nsTreeBodyFrame::PaintText
*PaintText also miscalculates the width of the text (untested in RTL)
*PaintText forces the text to be right-aligned when RTL is in effect

Comment 3

[(no longer active)]

Thanks Neil for your patch!

The other Neil will be on vacation for quite some time.  Is there anybody else who can review this?  roc maybe?

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 527681 [details] [diff] [review]
Proposed patch

Review of attachment 527681 [details] [diff] [review]:

Comment 5

[[e-mail]]

Pushed changeset 3bee3fefdec8 to mozilla-central.

Comment 6

[Firefox Bug Husbandry Bot]

Moving to Core:XUL per https://bugzilla.mozilla.org/show_bug.cgi?id=1455336