Closed
Bug 441452
Opened 16 years ago
Closed 16 years ago
Native Windows theme support needs to be able to handle mirrored RTL drawing
Categories
(Core :: Widget: Win32, defect)
Tracking
()
RESOLVED
FIXED
mozilla1.9.1a1
People
(Reporter: kliu, Assigned: kliu)
References
Details
(Keywords: fixed1.9.0.2, rtl)
Attachments
(1 file, 1 obsolete file)
3.26 KB,
patch
|
vlad
:
review+
vlad
:
superreview+
samuel.sidler+old
:
approval1.9.0.2+
|
Details | Diff | Splinter Review |
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.)
Attachment #326428 -
Flags: superreview?(roc)
Attachment #326428 -
Flags: review?(vladimir)
Flags: wanted1.9.1?
Flags: wanted1.9.0.x?
Summary: Native Windows theme support needs to be able to handled mirrored RTL drawing → Native Windows theme support needs to be able to handle mirrored RTL drawing
It's legal for the clip rect to be null.
Attachment #326428 -
Attachment is obsolete: true
Attachment #326685 -
Flags: superreview?(roc)
Attachment #326685 -
Flags: review?(vladimir)
Attachment #326428 -
Flags: superreview?(roc)
Attachment #326428 -
Flags: review?(vladimir)
Attachment #326685 -
Flags: superreview?(roc) → superreview?(vladimir)
Attachment #326685 -
Flags: superreview?(vladimir)
Attachment #326685 -
Flags: superreview+
Attachment #326685 -
Flags: review?(vladimir)
Attachment #326685 -
Flags: review+
Any way to create some tests for this?
(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.
Hmm.. you could at least check that the RTL result is different than the LTR result, no?
(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•16 years ago
|
||
Pushed to mozilla-central: http://hg.mozilla.org/mozilla-central/index.cgi/rev/8d870284418a
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Thanks for the push, Rob. With your theme reorg landed, do you think it would be better if this lived in nsUXThemeData instead?
Attachment #326685 -
Flags: approval1.9.0.2?
Comment 8•16 years ago
|
||
Comment on attachment 326685 [details] [diff] [review] patch v1.1 Approved for 1.9.0.2. Please land in CVS. a=ss
Attachment #326685 -
Flags: approval1.9.0.2? → approval1.9.0.2+
Updated•16 years ago
|
Flags: wanted1.9.0.x?
Flags: wanted1.9.0.x+
Flags: blocking1.9.0.2+
Keywords: checkin-needed
Updated•16 years ago
|
Product: Core → SeaMonkey
Component: Themes → Widget: Win32
Product: SeaMonkey → Core
Target Milestone: mozilla1.9.1a1 → ---
Checked into 1.9.0 branch.
Keywords: checkin-needed → fixed1.9.0.2
You need to log in
before you can comment on or make changes to this bug.
Description
•