204531 - rtl selectbox appears empty (probably css "width" related)

Status: RESOLVED FIXED

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

Description

[Thamer Mahmoud]

Gecko/20030429

The rtl select box on the 2nd line in URL is empty. The only exception is the
initial (or first) option. It is present inside the selectbox but NOT in the
white box that shows the full list of available options.
 
This bug is not reproducible on older builds (e.g. 20030407)

On a side note, The url is a testcase from Bug 203353.


1. go to URL
2. rtl select box on the 2nd line has zero elements

Comment 1

[Boris Zbarsky [:bzbarsky]]

roc, this sounds like you.  :(

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yep. Constrained-width RTL comboboxes are broken.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Note that my change did fix one RTL bug: scrollbars now appear on the left side
of the dropdown, when they appear at all.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

This patch fixes two bugs. The main thing it does is remove the BIDI hack that
reflows the scrollframe with a hacked width for RTL listboxes/comboboxes. I'm
not sure what that was for, since it's not commented, but it does not seem to
be necessary, and removing it fixes the bug. Certainly nsGfxScrollFrame is more
Bidi friendly than nsScrollFrame ever was, plus the testcases look good.
Perhaps smontagu can tell me why this code was there in the first place?

This patch also fixes a small bug where borders and padding were sometimes not
being included in the desired size for the listbox/combobox dropdown. This
shows up in the "constrained width" comboboxes in the testcase I'm about to
attach.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: testcase

This is like the previous testcase (which was excellent, thanks!) but adds
listbox versions of all the tests.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I'm *guessing* that BIDI code was supposed to move RTL content to the left when
a vertical scrollbar was added by nsScrollFrame. I have no idea why that was
needed. It's certainly not needed now.

Comment 7

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

Comment on attachment 123238 [details] [diff] [review]
fix

>+  // The nsGfxScrollFrame::Reflow adds border and padding to the

You could drop the "The " while you're reformatting this.

Comment 8

[Simon Montagu :smontagu]

Comment on attachment 123238 [details] [diff] [review]
fix

Originally we needed the extra reflow to align the items correctly in
right-to-left select boxes, plus what you said about adjusting for the
scrollbar. If it's now unnecessary, fine.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 123238 [details] [diff] [review]
fix

without this patch, RTL (e.g. Hebrew) comboboxes with constrained width are
totally horked.

Comment 10

[Brendan Eich [:brendan]]

Comment on attachment 123238 [details] [diff] [review]
fix

a=[e-mail] for 1.4final.

/be

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This was checked in a while ago

Comment 12

[(no longer active)]

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