Closed Bug 478377 Opened 15 years ago Closed 15 years ago

Support specifying direction for :-moz-tree-cell pseudo class through CSS

Categories

(Core :: XUL, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla1.9.2a1

People

(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)

References

Details

(Keywords: rtl, verified1.9.1)

Attachments

(1 file, 3 obsolete files)

Currently, if a tree body frame is RTL, it shows all its cells in RTL (the same goes for LTR).  Occasionally we need RTL data displayed in LTR trees (and vice versa).  I have a patch ready to add support for the CSS direction rule to the tree cells, which I will attach here together with a number of reftests.
Attached patch Patch (v1) (obsolete) — Splinter Review
Patch with reftest.
Attachment #362197 - Flags: superreview?(roc)
Attachment #362197 - Flags: review?(roc)
Keywords: rtl
+      if (aDirection == NS_STYLE_DIRECTION_INHERIT)
+        aDirection = aFrame->GetStyleVisibility()->mDirection;

{} around the statement

+  if (!((isRTL && aTextRTL) || (!isRTL && !aTextRTL)))

Can't this just be "isRTL != aTextRTL"?
Attached patch Patch (v1.1) (obsolete) — Splinter Review
Addressed comments
Attachment #362197 - Attachment is obsolete: true
Attachment #362209 - Flags: superreview?(roc)
Attachment #362209 - Flags: review?(roc)
Attachment #362197 - Flags: superreview?(roc)
Attachment #362197 - Flags: review?(roc)
Attached patch Patch (v1.1) (obsolete) — Splinter Review
Attachment #362209 - Attachment is obsolete: true
Attachment #362210 - Flags: superreview?(roc)
Attachment #362210 - Flags: review?(roc)
Attachment #362209 - Flags: superreview?(roc)
Attachment #362209 - Flags: review?(roc)
Blocks: 478430
+  if (isRTL != aTextRTL) {
+    direction = aTextRTL ? NS_STYLE_DIRECTION_RTL :
+                           NS_STYLE_DIRECTION_LTR;
+  }

Actually, why can't this be unconditional?
(In reply to comment #5)
> +  if (isRTL != aTextRTL) {
> +    direction = aTextRTL ? NS_STYLE_DIRECTION_RTL :
> +                           NS_STYLE_DIRECTION_LTR;
> +  }
> 
> Actually, why can't this be unconditional?

Hmmm, I think it can, because if direction is not set here, then nsLayoutUtils::DrawString will obtain the frame direction on its own, which is the same thing as |isRTL|.
Attached patch Patch (v1.2)Splinter Review
Attachment #362210 - Attachment is obsolete: true
Attachment #362372 - Flags: superreview?(roc)
Attachment #362372 - Flags: review?(roc)
Attachment #362210 - Flags: superreview?(roc)
Attachment #362210 - Flags: review?(roc)
Attachment #362372 - Flags: superreview?(roc)
Attachment #362372 - Flags: superreview+
Attachment #362372 - Flags: review?(roc)
Attachment #362372 - Flags: review+
http://hg.mozilla.org/mozilla-central/rev/247c0b3ec1aa
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
Attachment #362372 - Flags: approval1.9.1?
Comment on attachment 362372 [details] [diff] [review]
Patch (v1.2)

This patch comes with a test, and is needed for bugs 477902 and 478430 for 1.9.1.  Requesting approval.
Comment on attachment 362372 [details] [diff] [review]
Patch (v1.2)

a1.9.1=dbaron
Attachment #362372 - Flags: approval1.9.1? → approval1.9.1+
Verified fixed with reftests on OS X and Windows:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090301 Minefield/3.2a1pre ID:20090301020403

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090301 Shiretoko/3.1b3pre ID:20090301020400
Status: RESOLVED → VERIFIED
Blocks: 492250
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: