Closed Bug 1344395 Opened 8 years ago Closed 8 years ago

Checkboxes and radios on Android don't seem to have a zero width when width:auto is used

Categories

(Firefox for Android Graveyard :: Theme and Visual Design, defect)

Product:
Firefox for Android Graveyard
Component:
Theme and Visual Design
Platform:
All
Android
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox51 unaffected, firefox52 unaffected, firefox-esr52 unaffected, firefox53 unaffected, firefox54 fixed, firefox55 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 55
Tracking Flags:
Tracking Status
firefox51 --- unaffected
firefox52 --- unaffected
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: wisniewskit, Assigned: MatsPalmgren_bugz)

References

Details

(Keywords: regression, testcase, Whiteboard: [webcompat])

Attachments

(1 file)

fix
2.41 KB, patch
dholbert
: review+
gchang
: approval-mozilla-aurora+
Details | Diff | Splinter Review 
On Android, giving a checkbox or radio width:auto seems to collapse them, as though they have no intrinsic width anymore. A simple test-case show this:

<!DOCTYPE html>
<html>
<body>
With width:auto
<input type="checkbox" style="width:auto">
<input type="radio" style="width:auto">
Without width:auto
<input type="checkbox">
<input type="radio">
</body>
</html>


Based on a regression range found on webcompat.com (in the see-also link), I suspect this is related to bug 605985, so I've tentatively set that as a dependency.
Flags: needinfo?(mats)
Summary: Checkboxes and radios on Android no seem to have a zero width when width:auto is used → Checkboxes and radios on Android don't seem to have a zero width when width:auto is used
Not yet try on a local build without mats patches, but from nightly builds[1,2], it looks it's the case. 

OK: https://ftp.mozilla.org/pub/mobile/nightly/2017/02/2017-02-09-11-01-55-mozilla-central-android-x86/
NG: https://ftp.mozilla.org/pub/mobile/nightly/2017/02/2017-02-13-00-46-19-mozilla-central-android-x86/

Note that, in OK build, the checkbox is not checkable and its width is narrower that intrinsic size.
Blocks: 605985
No longer depends on: 605985
Right, I should have #ifdef'ed out those changes more thoroughly for Android/Gonk.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=54db3a8e9a13a3e2026e778d0fa7eb776071d567
Assignee: nobody → mats
Flags: needinfo?(mats)
Keywords: testcase
OS: Unspecified → Android
Hardware: Unspecified → All
Attached patch fix — — Splinter Review 
I'll try writing a reftest for this as well...
Attachment #8844978 - Flags: review?(dholbert)
Attachment #8844978 - Flags: review?(dholbert) → review+
Thanks, dholbert! I just verified that this patch makes my testcase in comment 0 work as it does in 51.0.3 on my phone.

However, the results do confuse me. In my testcase in comment 0, the box/radio that have a width specified end up being narrower than the ones with no width specified (as in, the radio is an oval). But this is what the code has always been done, and I'm not sure whether this is something that should be fixed:

>  static nscoord DefaultSize()
>  {
>    // XXXmats We have traditionally always returned 9px for GetMin/PrefISize
>    // but we might want to factor in what the theme says, something like:
>    // GetMinimumWidgetSize - GetWidgetPadding - GetWidgetBorder.
>    return nsPresContext::CSSPixelsToAppUnits(9);
>  }
Yeah, I think that's always been a bug on Android.
We need to re-implement checkbox/radio on Android anyway to be web-compatible
so I don't think it's worth tweaking this patch further.
Pushed by mpalmgren@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/983cc3810fbb
Make checkbox/radio controls have a default (non-zero) size also when -moz-appearance:none on Android/Gonk (since that's the default on these platforms).  r=dholbert
https://hg.mozilla.org/integration/mozilla-inbound/rev/5c05272150eb
Reftest for checkbox/radio controls with width/height:auto.
Comment on attachment 8844978 [details] [diff] [review]
fix

Approval Request Comment
[Feature/Bug causing the regression]: bug 605985
[User impact if declined]:minor layout issue with checkbox/radio controls on Android
[Is this code covered by automated tests?]:yes
[Has the fix been verified in Nightly?]:no
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]:no
[Why is the change risky/not risky?]:just some #ifdef's to revert some code from 605985 on Android
[String changes made/needed]:none
Attachment #8844978 - Flags: approval-mozilla-aurora?
(In reply to Mats Palmgren (:mats) from comment #5)
> Yeah, I think that's always been a bug on Android.
> We need to re-implement checkbox/radio on Android anyway to be web-compatible
> so I don't think it's worth tweaking this patch further.

Mats, do we already have a bug for it ?
Flags: needinfo?(mats)
https://hg.mozilla.org/mozilla-central/rev/983cc3810fbb
https://hg.mozilla.org/mozilla-central/rev/5c05272150eb
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
(In reply to Astley Chen [:astley] (UTC+8) from comment #8)
> Mats, do we already have a bug for it ?

We're discussing it in bug 1340661, I'm hoping Mike could pick it up
and fix it there.  If not, I'll make sure to file a new bug.
Flags: needinfo?(mats)
Comment on attachment 8844978 [details] [diff] [review]
fix

Fix a layout regression on Android. Aurora54+.
Attachment #8844978 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: in-testsuite+
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: