Options inside optgroup should show up as indented in select helper popup

VERIFIED FIXED in Firefox 11

Status

()

Firefox for Android
General
P4
normal
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: Martijn Wargers (dead), Assigned: wesj)

Tracking

({testcase})

unspecified
Firefox 12
ARM
Android
testcase
Points:
---

Firefox Tracking Flags

(firefox11- fixed, firefox12+ fixed)

Details

(URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

6 years ago
See testcase from url, which comes from bug 685197.

As you can see, the options "Three", "Four", "Five" are indented, because they are inside the "test" optgroup.
However, they don't show that way in the select helper popup (the popup that appears when tapping on the select).
The stock browser does show them as indented.
(Reporter)

Updated

6 years ago
Summary: Options inside optgroupd should show up as indented in select helper popup → Options inside optgroup should show up as indented in select helper popup

Updated

6 years ago
Assignee: nobody → wjohnston
tracking-firefox11: --- → -
tracking-firefox12: --- → +
Priority: -- → P4
(Assignee)

Comment 1

6 years ago
Created attachment 589535 [details]
Screenshot

Ahh. Yeah, I don't really like how the stock browser presents these, so I wasn't trying to match their style. But we do need to do something so that users can tell which items are in the optgroup, and which aren't.
(Assignee)

Comment 2

6 years ago
Created attachment 591196 [details] [diff] [review]
Patch

Simple solution. We could do a separate layout or something if we think its needed?
Attachment #591196 - Flags: review?(sriram)
Comment on attachment 591196 [details] [diff] [review]
Patch

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

setPadding's unit is pixels and not in dpi. Using pixels and not taking dpi into account makes me scary.
Please see if an expandable list view can do the trick. If not, try using "density" from "DeviceManager" metrics.
Attachment #591196 - Flags: review?(sriram) → review-
(Assignee)

Comment 4

6 years ago
Created attachment 591219 [details] [diff] [review]
Patch v2

Ahh. Good catch. So this does the conversion from dip to pixels for us. I looked at using an ExpandableListView instead. That would give us the ability to open and close groups of items, but wouldn't automatically fix this.

We'd have to create a special layout for these items, and to do it right we'd have to implement it for single select lists, multiselect lists, and context menu like lists. I don't want to have to manage separate layouts for every permutation possible here I think.
Attachment #591196 - Attachment is obsolete: true
Attachment #591219 - Flags: review?(sriram)
Comment on attachment 591219 [details] [diff] [review]
Patch v2

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

This looks good to me.
Attachment #591219 - Flags: review?(sriram) → review+
(Assignee)

Comment 6

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/17f835ac21d3
Whiteboard: [inbound]
https://hg.mozilla.org/mozilla-central/rev/17f835ac21d3
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → Firefox 12
(Assignee)

Comment 8

6 years ago
Comment on attachment 591219 [details] [diff] [review]
Patch v2

[Approval Request Comment]
Regression caused by (bug #): bug 695485
User impact if declined: ui can not make sense at times
Testing completed (on m-c, etc.): landed jan 24
Risk to taking this patch (and alternatives if risky): very low. this is odd to run into anyway
Attachment #591219 - Flags: approval-mozilla-aurora?
(Reporter)

Comment 9

6 years ago
Verified fixed in current Native trunk build.
Status: RESOLVED → VERIFIED
Comment on attachment 591219 [details] [diff] [review]
Patch v2

[Triage Comment]
Mobile only - approved for Aurora.
Attachment #591219 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/d8a62bf6787c
status-firefox11: --- → fixed

Updated

6 years ago
status-firefox12: --- → fixed
You need to log in before you can comment on or make changes to this bug.