Closed Bug 489517 Opened 16 years ago Closed 16 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 (deleted) —
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 (deleted) —
Flags: blocking1.9.1?
Attached patch Patch (obsolete) (deleted) — 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.
Attached patch Move it to DidSetStyleContext (deleted) — Splinter Review
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.
Status: NEW → RESOLVED
Closed: 16 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?
Priority: -- → P2
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: