Comment 38 for bug 288111

(In reply to comment #26)
> I don't follow this patch really. It seems that getColumnAtX should already
> return the right column whether in ltr or rtl mode, especially as you say you
> are interpreting 'before' or 'after' without respect to the direction.

OK, let me clarify.

_getColumnAtX's input (aX) is in physical coordinates (i.e., from the left side for both RTL and LTR modes). However, the tree column APIs work in logical ordering (i.e., the first column is the leftmost and rightmost for LTR and RTL, respectively). This is why we have to iterate on the columns from the end to beginning in RTL mode, so that we would effectively iterate over them in physical order. This way the existing formula for calculating the result column would continue to work.

Also, aPos is in logical order, that's why I swap its value for RTL iterations.

> Perhaps you really want to just iterate the same in either direction but the
> two lines here just be reversed?
>
> currentX += cw;
> if (currentX - (cw * aThresh) > adjustedX)

No, that won't work, because for example on the first iteration of the loop, cw is the width of the right-most (i.e., first) column, while currentX is the x position from the left side, so swapping those two lines isn't going to solve anything.

Given this description, what do you think about the patch?