Checkbox has 3 states – empty / full (white background) / with check-mark.

RESOLVED FIXED in Firefox 57

Status

()

Core
Widget: Android
--
major
RESOLVED FIXED
3 months ago
2 months ago

People

(Reporter: karlcow, Assigned: louis)

Tracking

({regression})

57 Branch
mozilla57
ARM
Android
regression
Points:
---
Dependency tree / graph
Bug Flags:
webcompat ?

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox55 unaffected, firefox56 unaffected, firefox57+ fixed)

Details

(Whiteboard: [webcompat], URL)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

3 months ago
Probably since the release of Bug 1352238 (Native theme for form controls on Firefox Android), a new issue has emerged on Firefox Android.


(needs window regression)

Steps to Reproduce:

Navigate to: http://m.focus.de/.
Tap the “User Account” icon.
Tap the "Jetzt registrieren" (“Register now”) button.
Scroll down the page to the options under the registration form.
Tap one of the option a few times and observe behavior.
Expected Behavior:
Check-box has 2 states – empty / with check-mark.

Actual Behavior:
Check-box has 3 states – empty / full (white background) / with check-mark.
Flags: webcompat?
(Reporter)

Updated

3 months ago
Summary: Check-box has 3 states – empty / full (white background) / with check-mark. → Checkbox has 3 states – empty / full (white background) / with check-mark.

Updated

3 months ago
Flags: needinfo?(lochang)
status-firefox56: --- → unaffected
tracking-firefox57: --- → ?
Keywords: regression
(Assignee)

Comment 1

3 months ago
I will investigate the bug. Leave the ni to remind myself.
(Reporter)

Updated

3 months ago
webcompat regression in 57, tracking.
Blocks: 1352238
tracking-firefox57: ? → +
No longer depends on: 1352238

Comment 3

3 months ago
Hmm, this is a surprising regression.  I don't see how adding a theme
could have changed the control states like this.

I'd be interested in knowing what the values of 'eventState.mStates' are here:
http://searchfox.org/mozilla-central/rev/2c9a5993ac40ec1db8450e3e0a85702fa291b9e2/widget/android/nsNativeThemeAndroid.cpp#171
(Assignee)

Updated

3 months ago
Assignee: nobody → lochang
Flags: needinfo?(lochang)
(Assignee)

Comment 4

3 months ago
I think this is the same issue with bug 1399776 that we do not paint the default background-color: white for checkbox. I have tested with the patch on bug 1399776 and it looks normal.
But still looking why the area of dotted line is strange.

(In reply to Mats Palmgren (:mats) from comment #3)
> Hmm, this is a surprising regression.  I don't see how adding a theme
> could have changed the control states like this.
> 
> I'd be interested in knowing what the values of 'eventState.mStates' are
> here:
> http://searchfox.org/mozilla-central/rev/
> 2c9a5993ac40ec1db8450e3e0a85702fa291b9e2/widget/android/nsNativeThemeAndroid.
> cpp#171

Here I list out all the events that I think are relevant to the issue in three cases:
1) If we click on an empty checkbox, we will first receive NS_EVENT_STATE_ACTIVE then receive NS_EVENT_STATE_FOCUS, NS_EVENT_STATE_HOVER and NS_EVENT_STATE_CHECKED.
2) If we unfocus the checkbox, we will receive nothing.
3) If we click on the checked checkbox, we will receive NS_EVENT_STATE_ACTIVE then receive NS_EVENT_STATE_FOCUS and NS_EVENT_STATE_HOVER.

Comment 5

3 months ago
(In reply to Louis Chang [:louis] from comment #4)
> I think this is the same issue with bug 1399776 that we do not paint the
> default background-color: white for checkbox. I have tested with the patch
> on bug 1399776 and it looks normal.

OK, sounds good.
Depends on: 1399776
Keywords: regressionwindow-wanted
(Assignee)

Comment 6

3 months ago
Except the strange area of dotted line, I think there is still something wrong with state handling. Since currently we paint grey hover color when state is NS_EVENT_STATE_HOVER. So when we try to unfocus a checked checkbox, the background color will change from grey to white. And if we click again it will become a grey but unchecked checkbox. Maybe we should paint the grey background color only when the state is NS_EVENT_STATE_CHECKED?

Besides, I'm curious what's the purpose of hover event on mobile phone? Since on mobile phone we have nothing to hover (unless using emulator...). Maybe we don't need to handle hover event?
Flags: needinfo?(mats)

Comment 7

3 months ago
Yeah, I think we can just ignore NS_EVENT_STATE_HOVER.

Or we could try changing it to NS_EVENT_STATE_ACTIVE instead?
http://searchfox.org/mozilla-central/rev/1c13d5cf85f904afb8976c02a80daa252b893fca/widget/android/nsNativeThemeAndroid.cpp#48,135
I think that would give feedback that the control is pressed.
I see that's how we paint it on OSX for example:
0. default state: white background (no checkmark)
1. press mouse button -> grey background (no checkmark)
2. release mouse button -> blue background + checkmark
3. press mouse button again -> darker blue background + checkmark
4. release mouse button -> default state

Whatever you think looks best is fine with me.
Flags: needinfo?(mats)

Comment 8

3 months ago
(In reply to Louis Chang [:louis] from comment #4)
> 2) If we unfocus the checkbox, we will receive nothing.

Do you mean that after clicking on a checkbox (to make it focused),
then clicking somewhere on the page background does not unfocus it?
That sounds a bit odd.

(In reply to Louis Chang [:louis] from comment #6)
> Except the strange area of dotted line, 

Are you referring to the dotted outline from our :-moz-focusring rule?
What's strange about it?
Flags: needinfo?(lochang)

Comment 9

3 months ago
(Not sure if the active state is worth signalling for checkbox/radio buttons
on a phone though, since presumably the user's finger is covering most/all
of the button anyway when pressing it.)
(In reply to Louis Chang [:louis] from comment #6)
> Besides, I'm curious what's the purpose of hover event on mobile phone?
> Since on mobile phone we have nothing to hover (unless using emulator...).
> Maybe we don't need to handle hover event?

The user could be using a mouse or a stylus that supports hovering, but maybe we don't need to indicate that visually.
(Assignee)

Comment 11

3 months ago
(In reply to Mats Palmgren (:mats) from comment #8)
> Do you mean that after clicking on a checkbox (to make it focused),
> then clicking somewhere on the page background does not unfocus it?
> That sounds a bit odd.

Not really. What I mean is when we click somewhere outside the checkbox we will receive a eventState but not includes NS_EVENT_STATE_ACTIVE,  NS_EVENT_STATE_HOVER, NS_EVENT_STATE_FOCUS. (Which will cause the repaint of checkbox without hover background color and result in checked checkbox with white background color)

> Are you referring to the dotted outline from our :-moz-focusring rule?
> What's strange about it?

Yeah, the focusring rule: http://searchfox.org/mozilla-central/source/layout/style/res/forms.css#593.
The strange rectangle area as shown in the picture of https://webcompat.com/issues/9864. I guess it's somehow related to bug 1400050.
Flags: needinfo?(lochang)
(Assignee)

Comment 12

3 months ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f7c85aca8f9e3bd2b02730e71c39703b667a5be5
Comment hidden (mozreview-request)
(Assignee)

Comment 14

3 months ago
(In reply to Mats Palmgren (:mats) from comment #7)
> Or we could try changing it to NS_EVENT_STATE_ACTIVE instead?

Ya, so I think this is good enough for now.
So I will first fix the grey background issue in this bug and fix the focusring issue in bug 1400050 (if it is the same problem).

Comment 15

3 months ago
mozreview-review
Comment on attachment 8910179 [details]
Bug 1399723 - Draw a grey background color when event state is active instead of hover.

https://reviewboard.mozilla.org/r/181672/#review187174

::: commit-message-a0eb2:1
(Diff revision 1)
> +Bug 1399723 - Draw a gery background color when state is active instead of hover. r?mats

s/gery/grey/
Attachment #8910179 - Flags: review?(mats) → review+
Comment hidden (mozreview-request)
(Assignee)

Updated

3 months ago
Keywords: checkin-needed

Comment 17

3 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/4a59e79b7e94
Draw a grey background color when event state is active instead of hover. r=mats
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/4a59e79b7e94
Status: NEW → RESOLVED
Last Resolved: 3 months ago
status-firefox57: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
status-firefox55: --- → unaffected
status-firefox-esr52: --- → unaffected
(Reporter)

Comment 19

3 months ago
mats, louis,

another 3 states issues which is not resolved by the latest build.
https://webcompat.com/issues/6220

It seems to be css related, but we didn't nail it down yet.
Flags: needinfo?(mats)
Flags: needinfo?(lochang)
(Assignee)

Comment 20

3 months ago
Ok, I can reproduce the problem on my physical machine. But I think it is different issue with the webcompat issues listed here (9808, 9816, 9803, 9901, 9864). Still looking for the root cause.
Flags: needinfo?(lochang)
Karl, could you open a new bug for tracking?
(In reply to Astley Chen [:astley] (UTC+8) from comment #21)
> Karl, could you open a new bug for tracking?

(ni? Karl for Comment #21)
Flags: needinfo?(kdubost)
(Reporter)

Updated

3 months ago
See Also: → bug 1403410
(Reporter)

Comment 23

3 months ago
Yup. Doing that now.
Opened Bug 1403410.

Thanks.
(Reporter)

Updated

3 months ago
Flags: needinfo?(kdubost)

Updated

2 months ago
Flags: needinfo?(mats)
You need to log in before you can comment on or make changes to this bug.