Last Comment Bug 652007 - Painting artifacts with RTL trees
: Painting artifacts with RTL trees
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: XP Toolkit/Widgets: XUL (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: neil@parkwaycc.co.uk
:
Mentors:
Depends on: 654017
Blocks: 649840
  Show dependency treegraph
 
Reported: 2011-04-21 16:28 PDT by neil@parkwaycc.co.uk
Modified: 2011-05-01 14:12 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Proposed patch (3.85 KB, patch)
2011-04-21 16:36 PDT, neil@parkwaycc.co.uk
roc: review+
Details | Diff | Splinter Review

Description neil@parkwaycc.co.uk 2011-04-21 16:28:30 PDT
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 neil@parkwaycc.co.uk 2011-04-21 16:36:22 PDT
Created attachment 527681 [details] [diff] [review]
Proposed patch
Comment 2 neil@parkwaycc.co.uk 2011-04-21 16:41:39 PDT
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 :Ehsan Akhgari (away Aug 1-5) 2011-04-28 19:25:14 PDT
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) (Exited; email my personal email if necessary) 2011-04-28 19:40:53 PDT
Comment on attachment 527681 [details] [diff] [review]
Proposed patch

Review of attachment 527681 [details] [diff] [review]:
Comment 5 neil@parkwaycc.co.uk 2011-04-29 12:36:40 PDT
Pushed changeset 3bee3fefdec8 to mozilla-central.

Note You need to log in before you can comment on or make changes to this bug.