903327 - Multiple selection dropdown listboxes are too small

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[u421692]

Attachment: aurora25

Environment:
Device: LG Nexus 4(Android 4.2.2)/HTC Desire HD(Android 2.3.5)
Build: Nightly 26.0a1(2013-08-08)/Aurora 25.0a2(2013-08-08)

Steps to reproduce:
1. Go to bugzilla.mozilla.org
2. Tap on "Advanced Search" tab
3. Observe the multiple selection listboxes

Expected result:
The multiple selection listboxes are sized correctly and have straight corners.(see attached screenshot)

Actual result:
The multiple selection listboxes are small and have rounded corners.(see attached screenshot)

Note:
It might be that the specification were changed, but I didn't find the bug related to this change.

Comment 1

[u421692]

Attachment: beta24

Comment 2

[Aaron Train [:aaronmt]]

font-inflation issue?

Comment 3

[Scott Johnson (:jwir3)]

(In reply to Aaron Train [:aaronmt] from comment #2)
> font-inflation issue?

Hm, I don't know about the rendering differences (i.e. the rounded vs. straight corners), but I'll look and see if something changed between the two versions that might have affected font inflation calculations.

Comment 4

[u421692]

Changing font size does not influence the size of the list boxes.

Comment 5

[Teodora Vermesan (:TeoVermesan)]

The regression window is:

mozilla-central
good build: 22.07.2013 
bad build: 22.07.2013 
-pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2268ff80683a&tochange=e3c19a339b36

Comment 6

[u421692]

We can not go deeper in inbound because there are no available builds for that time. However we can only guess that this Bug 896138 is the bug that caused the regression.

Comment 7

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

I'm skeptical about that regression range, and would be interested in seeing a range down to the changeset (which would be useful even if it is that bug).

Comment 8

[u421692]

After the changes on "https://bugzilla.mozilla.org/" the issue is not reproducing on this site on any Firefox build.
However, I was able to reproduce this issue on "http://www.w3schools.com/tags/tryit.asp?filename=tryhtml_select_multiple". 

The regression window is:

mozilla-central
good build: 04.07.2013 
bad build: 05.07.2013 
-pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=dcbbfcdf7bb4&tochange=17fe59f6c54a

Comment 9

[u421692]

I will start a hg bisect on this, since there are no builds in the inbound folder for that time.

Comment 10

[u421692]

The first bad revision is:
changeset:   137443:efa23d6bd3fa
user:        Kartikaya Gupta <[e-mail]>
date:        Thu Jul 04 09:02:29 2013 -0400
summary:     Bug 803207 - Kill GetDevicePixelsPerMetaViewportPixel and use widget scaling correctly on Fennec. r=mbrubeck

inbound pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=1947e3caa654&tochange=efa23d6bd3fa

It seems that Bug 803207 caused this issue.

Comment 11

[Kartikaya Gupta (email:[e-mail])]

I believe this has to do with the default styling we have for form elements in Fennec. On my Nexus 4, the following page looks different on Beta and in Aurora: http://people.mozilla.org/~kgupta/bug/903327.html

On Beta the two buttons are the same size but on Aurora they are not. If I change layout.css.devPixelsPerPx to 1.0 on the Aurora build (the default is 2.0 on the Nexus 4) then the buttons render the same size.

Based on this I believe the default styling we apply to form elements (and perhaps other parts of the page) are specified in CSS pixels, and so when bug 803207 landed and changed the default value of devPixelsPerPx on hi-dpi devices, it affected how large the form elements are rendered as.

I took a quick look at Fennec's content.css but didn't see which CSS properties might be affecting this. Wesj, do you know where our form elements pick up default properties regarding font size from?

Comment 12

[Wesley Johnston (:wesj)]

I don't think this font-size is specified in CSS. It comes from the eCSSUnit_System_Font keyword. I'll have to dig more to see where that's set for us.

Comment 13

[Wesley Johnston (:wesj)]

Attachment: Patch

Comment 14

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a253c1d64293

Comment 15

[Wesley Johnston (:wesj)]

Comment on attachment 803172 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 803207. We're correcting something that should have been done in bug 674373, but wasn't noticed until we made some later changes.
User impact if declined: Text fields using strange font sizes
Testing completed (on m-c, etc.): Landed on mc today. Tested locally on test pages.
Risk to taking this patch (and alternatives if risky): Low risk. Will change rendering of textboxes (hopefully to match their old display). I cant think of any alternatives. Back out the dev pixels change?
String or IDL/UUID changes made by this patch: None.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a253c1d64293

Comment 18

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/2e2866bc317e

Comment 19

[u421692]

Verified as fixed on:
Device: LG Nexus 4 (Android 4.2.2)
Build: Nightly 26.0a1 (2013-09-16) / Aurora 25.0a2 (2013-09-16)

Comment 20

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 803172 [details] [diff] [review]
Patch

Review of attachment 803172 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/android/nsLookAndFeel.cpp
@@ +452,5 @@
>      aFontName.AssignLiteral("\"Droid Sans\"");
>      aFontStyle.style = NS_FONT_STYLE_NORMAL;
>      aFontStyle.weight = NS_FONT_WEIGHT_NORMAL;
>      aFontStyle.stretch = NS_FONT_STRETCH_NORMAL;
> +    aFontStyle.size = 9.0 * 96.0f / 72.0f * aDevPixPerCSSPixel;

The number of magic numbers makes me sad.