1208978 - Firefox select tag bug with Hindi language

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect)

Description

[Alexander Berlin]

Attachment: Bug

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/45.0.2454.101 Safari/537.36

Steps to reproduce:

Use this code:


<select onchange="onChangeLang(this)" class="b-lang__select" style="height: 20px;">
  <option label="English" value="en">English</option>
  <option label="Deutsch" value="de">Deutsch</option>
  <option label="Français" value="fr">Français</option>
  <option label="Español" value="es">Español</option>
  <option label="Italiano" value="it">Italiano</option>
  <option selected="" label="Русский" value="ru">Русский</option>
  <option label="Chinese" value="zh">Chinese</option>
  <option label="Български" value="bg">Български</option>
  <option label="Ελληνικά" value="el">Ελληνικά</option>
  <option label="Suomi" value="fi">Suomi</option>
  <option label="हिंदी" value="hi">हिंदी</option>
  <option label="日本語" value="ja">日本語</option>
  <option label="Polski" value="pl">Polski</option>
  <option label="Português" value="pt">Português</option>
  <option label="Română" value="ro">Română</option>
  <option label="Српски" value="sr">Српски</option>
  <option label="Svenska" value="sv">Svenska</option>
  <option label="Türkçe" value="tr">Türkçe</option>
  <option label="Bahasa Indonesia" value="id">Bahasa Indonesia</option><option label="한국어" value="ko">한국어</option><option label="ไทย" value="th">ไทย</option>
</select>


Actual results:

Wrong height of dropdown list, when Hindi language in list.


Expected results:

Correct height of dropdown list.

Comment 1

[Jonathan Kew [:jfkthame]]

Experimenting a bit, I find that this is not specific to Hindi; I can reproduce a similar issue with a list that contains one instance of <option>العربي</option>, followed by 20 copies of <option>English</option>.

Moreover, if you have fewer options in the <select> (less than 20), the problem won't occur (even with Hindi present); and if you add several more copies of the Hindi option, so that a vertical scrollbar appears on the list, it won't occur then either.

The issue seems to arise when we get a combination of two factors:
(a) there are slightly over 20 items in the list (the example above has 21); and
(b) one or more of them trigger fallback to a font (such as Devanagari) that has greater default line-height.

Comment 2

[Jonathan Kew [:jfkthame]]

My guess is that what happens is something like this: when there are more than 20 items in the list, we intend to restrict its height to the height of 20 items and provide a scrollbar to get to the rest.

But when some of those items are using different (fallback) fonts, we get the size computation wrong: it looks like we calculate the height needed for 20 items based on the largest line-height of any of the fonts being used, and then we fix the vertical height of the popup to that size; but we still render each item with its font's default height.

In this case, that means that most of the items (all except the Hindi one, probably) are occupying less height than we allowed for, and so we have some vertical space left over.

Comment 3

[Jonathan Kew [:jfkthame]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=361748bf4f3a

Comment 4

[Jonathan Kew [:jfkthame]]

Attachment: Limit the size of a potentially-scrollable dropdown list to the actual size of its contents, as our estimate based on row count may be too large if the row heights vary

Comment 5

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e42955baebbd10675a488cc377d3d9a4ef785a24
Bug 1208978 - Limit the size of a potentially-scrollable dropdown list to the actual size of its contents, as our estimate based on row count may be too large if the row heights vary. r=roc

Comment 6

[Nigel Babu [:nigelb]]

You have build bustage!

Comment 7

[Nigel Babu [:nigelb]]

/builds/slave/m-in-l64-asan-0000000000000000/build/src/layout/forms/nsListControlFrame.cpp:590:22: error: no matching function for call to 'min'

Comment 8

[Nigel Babu [:nigelb]]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/4c2c0eef4d12

Comment 9

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/67f016207ddeab47ac1341873e6d3c1be389f0f5
Bug 1208978 - Limit the size of a potentially-scrollable dropdown list to the actual size of its contents, as our estimate based on row count may be too large if the row heights vary. r=roc

Comment 10

[Jonathan Kew [:jfkthame]]

Argh, silly signed-vs-unsignedness.... sorry! Fixed and relanded.

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/67f016207dde