421419 - Inconsistent layout with rtl, float, margin-left

Status: RESOLVED FIXED

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

Description

[Jesse Ruderman]

The testcase and reference should look the same because they have identical final DOMs.

Based on the reftest for bug 420069.

Comment 1

[Jesse Ruderman]

Attachment: testcase (dynamic)

Comment 2

[Jesse Ruderman]

Attachment: reference (static)

Comment 3

[Uri Bernstein]

It's pretty clear to me that the static reference is the one that's broken: the margin seems to be counted twice when calculating the width of the float.

Comment 4

[Uri Bernstein]

This is a regression from bug 365130. It seems that nsContainerFrame::DoInlineIntrinsicWidth() make some assumptions which are not true if bidi resolution already happened.

Comment 5

[Uri Bernstein]

Attachment: patch?

Don't assume the first-in-flow [last-in-flow] is on the first [last] line if it has a previous [next] continuation.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 307935 [details] [diff] [review]
patch?

r+sr=dbaron, but maybe add a comment mentioning bidi continuations (and please add the test as a reftest)

We should get this in for 1.9.  It's low risk; it affects padding/border/margin only on inline elements that have multiple text directions within them.

Comment 7

[Uri Bernstein]

Attachment: patch for checkin

Patch with extra comments and (slightly simplified) reftest.

Comment 8

[Damon Sicore (:damons)]

Comment on attachment 307935 [details] [diff] [review]
patch?

a1.9+=damons

Comment 9

[Uri Bernstein]

Checked in:
Checking in mozilla/layout/generic/nsContainerFrame.cpp;
/cvsroot/mozilla/layout/generic/nsContainerFrame.cpp,v  <--  nsContainerFrame.cpp
new revision: 1.311; previous revision: 1.310
done
Checking in mozilla/layout/reftests/bugs/reftest.list;
/cvsroot/mozilla/layout/reftests/bugs/reftest.list,v  <--  reftest.list
new revision: 1.394; previous revision: 1.393
done
RCS file: /cvsroot/mozilla/layout/reftests/bugs/421419-1-ref.html,v
done
Checking in mozilla/layout/reftests/bugs/421419-1-ref.html;
/cvsroot/mozilla/layout/reftests/bugs/421419-1-ref.html,v  <--  421419-1-ref.html
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/layout/reftests/bugs/421419-1.html,v
done
Checking in mozilla/layout/reftests/bugs/421419-1.html;
/cvsroot/mozilla/layout/reftests/bugs/421419-1.html,v  <--  421419-1.html
initial revision: 1.1
done

Comment 10

[(no longer active)]

Mass-assigning the new rtl keyword to RTL-related (see bug 349193).