839771 - Inconsistent rendering of option circles in select

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Peter Retzer (:pretzer)]

Attachment: Before scrolling

The circles next to options within a select element have changed their position slightly after they have been scrolled out of view an back into view. Before scrolling they have small margin on the right and after scrolling they show up right aligned. See also the screenshots to get what I mean. 
This is a regression in FF20. Earlier versions show them fine.

Comment 1

[Peter Retzer (:pretzer)]

Attachment: After scrolling

Comment 2

[Andreea Pod]

The regression window for this issue is:

good build:
2013/01/05

bad build 
2013/01/06

possible push-log:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d8ca3e1c469e&tochange=20d1a5916ef6

Comment 3

[Aaron Train [:aaronmt]]

Even though there are 160 change-sets there, bug 736321 stands out.

Comment 4

[Kevin Brosnan [Ex-Mozilla]]

Wes can you take a look at this?

Comment 5

[Wesley Johnston (:wesj)]

Attachment: Patch

This is a different way of solving this. Android draws the checkbox for the CheckedTextView on onDraw, meaning there is no "View" for the checkbox/radio button at the end. When we add padding, it tries to adjust, but it screws up quite frequently. In this case for some reason, when we initially create the views it adds the padding twice. As they're recycled, it corrects itself.

The only reason this padding is present is to indent items that are inside opt groups. This instead uses a "blank" compound drawable to add the padding, and doesn't seem to have these same issues. It also fixes a multitude of other bugs I've seen where these checkboxes are drawn in random incorrect places.

I also threw a fix in where we currently allow you to select items in disabled selects.

Comment 6

[Wesley Johnston (:wesj)]

Attachment: Screenshot

Screenshot from my test page at:

http://dl.dropbox.com/u/72157/forms.html

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

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

LGTM, but let's get Sriram to take a look

Comment 8

[Sriram Ramasubramanian [:sriram]]

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

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

Looks good to me.

::: mobile/android/base/PromptService.java
@@ +609,5 @@
>          }
>  
> +        private Drawable getMoreDrawable() {
> +            if (mMoreDrawable == null) {
> +                mMoreDrawable = res.getDrawable(android.R.drawable.ic_menu_more);

Where is this "res" coming from?

Comment 9

[Wesley Johnston (:wesj)]

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

Comment 10

[Ryan VanderMeulen [:RyanVM]]

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

Comment 11

[Andreea Pod]

Verified fixed on:
-build: Firefox for Android 22.0a1 (2013-03-12)
-device: Samsung Galaxy Nexus
-OS: Android 4.1.1