When selecting Hebrew as the UI language for Gmail, the layout is broken: The right sidebar is way too wide (occupying most of the viewport), and a horizontal scrollbar appears. This regressed between 2006-06-20-05 and 2006-06-21-05. Bonsai link: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2006-06-20+05%3A00&maxdate=2006-06-21+06%3A00&cvsroot=%2Fcvsroot

Seeing this on Linux also (but not Windows).

This shows that the problem is as following: After setting the content of a span to be a bidi string for the first time, measuring it using scrollWidth returns a bogus result, related to the width of the viewport (in this case, at least). Setting the content of the span again (even with the exact same value), fixes the problem.

(In reply to comment #2) > Setting the content of the span again > (even with the exact same value), fixes the problem. > Actually it doesn't. The second measurement is unrelated to the window width but it is still wrong (this can be verified by comparing it to the value given by pre-regression builds, or by direct measuring).

Bug 341711 is a possible candidate for causing this.

The patch in bug 365130 gresses this regression(so comment 1 may be inaccurate. I will retest in windows with a build without that patch shortly).

(In reply to comment #5) > The patch in bug 365130 gresses this regression(so comment 1 may be inaccurate. > I will retest in windows with a build without that patch shortly). > I just tested in a Windows build, and it's to be broken. Even if this is fixed by bug 365130, it might be worth taking a look into why it broke, IMO.

I'm pretty sure bug 341711 did, in fact, cause this. Simon - does your patch for bug 365130 fix this completely (i.e., both measurements are correct), or does it only fix the first measurement to return the same incorrect value as the second one?

Both measurements are correct.

We don't need to add the offset-to-parent to the continuation frame's rect. Simply taking the union of the rects for each of the frame and all its continuations, works.

Comment on attachment 253756 [details] [diff] [review] patch This isn't right, because the continuation frames may have different parents hence their GetRect()s may return rectangles in different coordinate systems, and should not be unioned together blindly. I think the real problem is that f->GetRect() + f->GetOffsetTo(parent) should be nsRect(f->GetOffsetTo(parent), f->GetSize()) so we don't include the offset from f to its parent twice...

You're right, of course, and this /really/ works. I'll also attach an alternative patch shortly, and let you decide.

This was inspired by the fact that I found the "relative to aFrame's origin" part unintuitive, and that neither of the current two callers care about anything other than the size of the rect. It slightly simplifies GetAllInFlowBoundingRect/GetAllInFlowUnionSize, but the information I'm removing might be useful to someone in the future.

Comment on attachment 254050 [details] [diff] [review] roc's suggestion Let's go with this one. It's hardly any more code.

Checking in mozilla/layout/base/nsLayoutUtils.cpp; /cvsroot/mozilla/layout/base/nsLayoutUtils.cpp,v <-- nsLayoutUtils.cpp new revision: 3.82; previous revision: 3.81 done

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