Closed Bug 489517 Opened 11 years ago Closed 11 years ago

Setting direction to rtl doesn't enable Bidi processing

Categories

(Core :: Layout: Text and Fonts, defect, P2)

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: smontagu, Assigned: smontagu)

References

Details

(4 keywords)

Attachments

(3 files, 1 obsolete file)

Attached file testcase
This is a regression from bug 441367 caused by the change to "direction" in nsRuleNode::ComputeVisibilityData, specifically removing:

-    if (NS_STYLE_DIRECTION_RTL == visibility->mDirection)
-      mPresContext->SetBidiEnabled();

Adding a similar line in nsCSSFrameConstructor presumably prevented this causing a regression in static pages, but it doesn't help when changing direction dynamically.
Attached file Reference rendering
Flags: blocking1.9.1?
Attached patch Patch (obsolete) — Splinter Review
Putting back that line fixes the testcase -- not yet tested otherwise.
Can we move it from nsCSSFrameConstructor to nsFrame::DidSetStyleContext instead?  For the dynamic change case I'm not sure if we're guaranteed to get the visibility struct and trigger this code.
Doing it that way fixes the regression and doesn't appear to cause any new regressions in reftests or layout mochitests.

Since the code I'm moving references bug 115921, I added a couple of reftests for that as well as the testcase here.
Attachment #374020 - Attachment is obsolete: true
Attachment #374045 - Flags: superreview?(dbaron)
Attachment #374045 - Flags: review?(dbaron)
Attachment #374045 - Flags: superreview?(dbaron)
Attachment #374045 - Flags: superreview+
Attachment #374045 - Flags: review?(dbaron)
Attachment #374045 - Flags: review+
Comment on attachment 374045 [details] [diff] [review]
Move it to DidSetStyleContext

r+sr=dbaron

It's probably better if the tests that show "No new line at end of file" in the diff have a newline at the end of the file.
http://hg.mozilla.org/mozilla-central/rev/c63c9e5b421c
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Flags: blocking1.9.1? → blocking1.9.1+
Given that I marked this blocking, could you land it on branch too?
You need to log in before you can comment on or make changes to this bug.