Closed Bug 1068027 Opened 6 years ago Closed 6 years ago

nsBidiPresUtils::RepositionFrame mixes logical and physical directions

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: jfkthame, Assigned: jfkthame)

Details

Attachments

(2 files)

In RepositionFrame, the code mixes frameWidth (physical) with the line-logical aStart and iCoord values. This leads to text-painting failures with bidi text in vertical writing modes.
This depends on the rest of the vertical-text patch queue before it's actually useful, but it appears to fix the problems in my local testing.
Attachment #8490085 - Flags: review?(smontagu)
FWIW, here's a simple testcase that exhibits bad behavior due to this bug, in a build with vertical writing modes enabled. Note how the layout in the <div>s with vertical WM overflows badly at the bottom. The patch here fixes this, so that the layout within these <div>s correctly wraps within the expected area.
Comment on attachment 8490085 [details] [diff] [review]
nsBidiPresUtils::RepositionFrame should not mix logical and physical directions.

Review of attachment 8490085 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/base/nsBidiPresUtils.cpp
@@ +1401,5 @@
>      aStart += margin.IStart(aLineWM);
>    }
>  
>    nscoord start = aStart;
> +  nscoord frameISize = LogicalSize(aLineWM, aFrame->GetSize()).ISize(aLineWM);

= aFrame->ISize(aLineWM)
Attachment #8490085 - Flags: review?(smontagu) → review+
(In reply to Simon Montagu :smontagu from comment #3)
> > +  nscoord frameISize = LogicalSize(aLineWM, aFrame->GetSize()).ISize(aLineWM);
> 
> = aFrame->ISize(aLineWM)

Oh yes, much more concise.

https://hg.mozilla.org/integration/mozilla-inbound/rev/2752d91bde33
Target Milestone: --- → mozilla35
https://hg.mozilla.org/mozilla-central/rev/2752d91bde33
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.