441452 - Native Windows theme support needs to be able to handle mirrored RTL drawing

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Kai Liu]

Attachment: RTL-aware DrawThemeBackground

We need a way to mirror DrawThemeBackground so that widgets that are not direction-neutral will look right in RTL.  SetLayout is troublesome because it reverses the coordinate system of the DC's entire drawing area, which means that in addition to mirroring the appearance, it also mirrors the position.

We could either try adjusting our rects to counter the position changes that SetLayout causes or we could create a separate DC with the same size as the widget rect, call SetLayout on that, and transfer the results over.

I think I finally figured out how to reliably and exactly counter the position mirroring caused by SetLayout, so this patch uses the first approach.  Since SetLayout is a black-box function, I can't be completely sure that this is correct, but this works with a number of different widget styles on both Vista and XP, and I have yet to find a problem with it in my testing.

(This patch only adds a RTL-aware wrapper for DrawThemeBackground; the actual use of this will be implemented by the various bugs that this bug blocks.)

Comment 1

[Kai Liu]

Attachment: patch v1.1

It's legal for the clip rect to be null.

Comment 2

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Any way to create some tests for this?

Comment 3

[Kai Liu]

(In reply to comment #2)
> Any way to create some tests for this?
> 

I manually tested this function by having every widget take the new path if the direction is RTL (instead of just having the direction-sensitive widgets take the path) and everything looked okay.  As for automated testing, I haven't been able to come up with a good idea on how to do that, except to hard-code the desired end result into a PNG for a reftest, but that seems like it'd be fragile.

Comment 4

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Hmm.. you could at least check that the RTL result is different than the LTR result, no?

Comment 5

[Kai Liu]

(In reply to comment #4)
> Hmm.. you could at least check that the RTL result is different than the LTR
> result, no?
> 

Sure, I could do that.  I've attached the test to bug 403458.

Comment 6

[Rob Arnold [:robarnold]]

Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/index.cgi/rev/8d870284418a

Comment 7

[Kai Liu]

Thanks for the push, Rob.  With your theme reorg landed, do you think it would be better if this lived in nsUXThemeData instead?

Comment 8

[Samuel Sidler (old account; do not CC)]

Comment on attachment 326685 [details] [diff] [review]
patch v1.1

Approved for 1.9.0.2. Please land in CVS. a=ss

Comment 9

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

Checked into 1.9.0 branch.