Closed Bug 839771 Opened 8 years ago Closed 8 years ago

Inconsistent rendering of option circles in select

Categories

(Firefox for Android :: General, defect)

20 Branch
All
Android
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 22
Tracking Status
firefox19 --- unaffected
firefox20 --- affected
firefox21 --- affected
firefox22 --- verified

People

(Reporter: pretzer, Assigned: wesj)

References

Details

(Keywords: regression)

Attachments

(4 files)

Attached image 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.
Attached image After scrolling
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
Even though there are 160 change-sets there, bug 736321 stands out.
Blocks: 736321
Wes can you take a look at this?
Flags: needinfo?(wjohnston)
Attached patch PatchSplinter Review
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.
Assignee: nobody → wjohnston
Attachment #715725 - Flags: review?(mark.finkle)
Flags: needinfo?(wjohnston)
Attached image Screenshot
Screenshot from my test page at:

http://dl.dropbox.com/u/72157/forms.html
Comment on attachment 715725 [details] [diff] [review]
Patch

LGTM, but let's get Sriram to take a look
Attachment #715725 - Flags: review?(mark.finkle) → review?(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?
Attachment #715725 - Flags: review?(sriram) → review+
https://hg.mozilla.org/mozilla-central/rev/e91be95894d1
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 22
Verified fixed on:
-build: Firefox for Android 22.0a1 (2013-03-12)
-device: Samsung Galaxy Nexus
-OS: Android 4.1.1
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.