1403410 - Checkbox has 3 states – empty / outlined / with check-mark.

Status: RESOLVED WORKSFORME

(GeckoView :: General, defect, P2)

Description

[Karl Dubost💡 :karlcow]

Attachment: Three states of the checkbox

This is a spin-off of https://webcompat.com/issues/6220


With Firefox on Android.
This is happening in Firefox 52, 55, 57 and 58 (at least).
This is NOT happening on Desktop (with responsive design and Firefox Android UA)


Steps to Reproduce

1. Navigate to: www.amazon.fr
2. Tap the Login button.
3. Tap on the "Show password" and "Keep me logged in" checkboxes to enable/disable them. Observe the behavior.

Expected Behavior:
The checkboxes are Checked/Not checked after one tap.

Actual Behavior:
The checkboxes need more than one tap to change state.

The screenshot in this bug is obtained by first doing


.a-control-row .a-checkbox input, .a-control-row .a-icon-checkbox, .a-control-row .a-icon-radio, .a-control-row .a-radio input {
	/* position: absolute; */
	left: .4rem;
	top: 50%;
	margin-top: -1.2rem;
}

.a-checkbox.a-checkbox-fancy input, .a-radio.a-radio-fancy input {
	/* opacity: .02; */
	border: 0 none;
	margin: 0;
	outline: 0 none;
	overflow: hidden;
	padding: 0;
	position: absolute;
	bottom: auto;
	left: 0;
	z-index: -1;
}

Basically there is an `i` element on top of the actual input checkbox. In the screenshots the real `input` is on the left side and the `i` on the right side

<div id="auth-show-password-checkbox-container" class="a-checkbox a-checkbox-fancy a-control-row a-touch-checkbox auth-show-password-checkbox">
  <label for="auth-show-password-checkbox" class="">
    <input id="auth-show-password-checkbox" name="" value="" checked="" tabindex="3" type="checkbox">
  <i class="a-icon a-icon-checkbox"></i>
  <span class="a-label a-checkbox-label">
          Show password
   </span>
  </label>
</div>


https://github.com/ghoo1125 found out that removing all the values on 

.a-checkbox.a-checkbox-fancy input, .a-radio.a-radio-fancy input {
/*
opacity: .02;
border: 0 none;
margin: 0;
outline: 0 none;
overflow: hidden;
padding: 0;
position: absolute;
bottom: auto;
left: 0;
z-index: -1;
*/
}

solves all the issues.

Comment 1

[Louis Chang [:louis]]

Attachment: testcases.html

The minimal test case that can reproduce the 3 states issue.

Comment 2

[Louis Chang [:louis]]

Attachment: four_states.png

Comment 3

[Louis Chang [:louis]]

Hi mats,

I look into the events we receive and it seems our painting codes work correctly according to the events we received.
So maybe we should figure out "why" we receive these events instead?

However, I list out events we receive for 3 states issue (actually 4 states, see attachment 8918191 [details]) below:
Click on first checkbox (Left most) will receive: 1, 2, 3, 5, 10, 12, 22, 29, 39, 43, 44
Click on second checkbox will receive           : 3, 5, 10, 12, 22, 29, 44
Click on third checkbox will receive            : 1, 2, 3, 10, 12, 22, 29, 39, 43, 44
Click on fourth checkbox will receive           : 3, 10, 12, 22, 29, 44

The event number is based on the definition here:
http://searchfox.org/mozilla-central/source/dom/events/EventStates.h#197-318

Do you have any idea where the problem is? Or where should I start looking from?

Comment 4

[Louis Chang [:louis]]

Note the correct behavior and the received events should be:
Click on first checkbox (Left most) will receive: 1, 2, 3, 5, 10, 12, 22, 29, 39, 43, 44
Click on third checkbox will receive            : 1, 2, 3, 10, 12, 22, 29, 39, 43, 44
and repeat.

Comment 5

[Mats Palmgren (inactive)]

I'm not sure I understand what "Click on first checkbox", "Click on second checkbox" etc means.
I'm guessing these are four calls to our painting code, with the given state flags for each,
that resulted from two separate clicks on the same checkbox, the first click toggled
the checkbox on, the second click toggled it off.  Is that correct?
(so this is roughly a "mousedown", "mouseup", "mousedown", "mouseup" sequence of events
on the same checkbox)

I'm also confused about the given numbers, are they bits in the state so "1" means
NS_EVENT_STATE_ACTIVE etc? or did you translate it so that 1 means NS_EVENT_STATE_FOCUS?

Looking at the screenshot, it appears to have a painted border in all cases,
except that it's grey in phase 3 and 5, which seems to be the problem.
PaintCheckboxControl unconditionally paints a border with sAndroidBorderColor
(orange) so I don't understand where that grey border comes from at all.
Can you move the border painting code after the background painting code
and see if that makes a difference?  What happens if you remove the code
for NS_EVENT_STATE_DISABLED/ACTIVE -- does that make a difference?
For debugging purposes it might help to temporarily change the colors to red,
blue, green etc to make them easier to distinguish.

Also, in the future, please describe more precisely what gestures you are doing
and which results you observe for each.

Comment 6

[Mats Palmgren (inactive)]

Actually, sAndroidBorderColor seems to be the grey one:
http://searchfox.org/mozilla-central/source/widget/android/AndroidColors.h#15
but then I don't understand where the orange border in the screenshot comes
from.  None of the colors in AndroidColors.h appears to be orange.

Comment 7

[Louis Chang [:louis]]

(In reply to Mats Palmgren (:mats) from comment #5)
Sorry for not being clear.

> I'm not sure I understand what "Click on first checkbox", "Click on second
> checkbox" etc means.
> I'm guessing these are four calls to our painting code, with the given state
> flags for each,
> that resulted from two separate clicks on the same checkbox, the first click
> toggled
> the checkbox on, the second click toggled it off.  Is that correct?
> (so this is roughly a "mousedown", "mouseup", "mousedown", "mouseup"
> sequence of events
> on the same checkbox)

So one click includes mouse down and mouse up. And click on the first checkbox means click on the left most checkbox in the picture (checkbox with only grey border), then we will receive the events (1, 2, 3, 5, 10, 12, 22, 29, 39, 43, 44) and paint out the second checkbox (checkbox with orange border and checkmark). Next, click on the second checkobx then we receive the events (3, 5, 10, 12, 22, 29, 44) and paint out the third checkbox. Finally, after clicking the fourth checkbox we go back to the first stage and repeat.

> I'm also confused about the given numbers, are they bits in the state so "1"
> means
> NS_EVENT_STATE_ACTIVE etc? or did you translate it so that 1 means
> NS_EVENT_STATE_FOCUS?

0 is NS_EVENT_STATE_ACTIVE, 1 is NS_EVENT_STATE_FOCUS and so on. So in this test case we do not receive any NS_EVENT_ACTIVE at all.

> Looking at the screenshot, it appears to have a painted border in all cases,
> except that it's grey in phase 3 and 5, which seems to be the problem.
> PaintCheckboxControl unconditionally paints a border with sAndroidBorderColor
> (orange) so I don't understand where that grey border comes from at all.
> Can you move the border painting code after the background painting code
> and see if that makes a difference?  What happens if you remove the code
> for NS_EVENT_STATE_DISABLED/ACTIVE -- does that make a difference?
> For debugging purposes it might help to temporarily change the colors to red,
> blue, green etc to make them easier to distinguish.

So I think the orange border is the border of i element in the test case. The orange border is painted due to the rule:
input:focus+.a-icon:after {
  border: 1px solid #e77600;
  content: ' ';
  position: absolute;
  top: 0;
  left: 0;
  height: 2.1rem;
  width: 2.1rem;
}
Also I think the border is not painted by our native theme. Actually there are still grey border painted by our native theme in second and fourth checkbox, but nearly cover by the orange border. However, the expected result should be we click on the first checkbox and paint out the second checkbox. Next click on the second checkbox and paint out first checkbox and repeat.

Comment 8

[Louis Chang [:louis]]

Acually, maybe the reduced test case is not correct... I have tested the reduced test case on other platforms including Firefox, Chrome on Linux, Mac and Chrome on mobile they could not paint out the checkmark at all. But only Firefox on mobile can paint out the checkmark and have the four state issue.

Comment 9

[Louis Chang [:louis]]

I will simplify the test case and update the result again.

Comment 10

[Louis Chang [:louis]]

Attachment: testcase.html

Comment 11

[Louis Chang [:louis]]

The reduced test case in comment 10 could work normally on other platforms including Firefox, Chrome on Linux, Mac and Chrome on Android. But there is a four states issue with Firefox on Android. Basically, the test case is to fake a checkbox by using <i> element with specific background images. The <i> element will change the background image based on the state changed of the real checkbox.

Expected behavior:
The checkbox should only have two states when we click on it:
checkbox with orange border and no checkmark,
checkbox with orange border and checkmark

Actual behavior:
The checkbox has four states as shown in attachment 8918191 [details]. So we have to click twice on the checkbox to check or uncheck the checkbox.

The events we received are basically the same as I mentioned in comment 5 and comment 7.
Also, if I remove this rule:
input:focus+i:after {
  content:' ';
}
Firefox on Android could work normally as well.

Do you have any thought? Feel free to ask if there is still something not clear enough.

Comment 12

[Mats Palmgren (inactive)]

(In reply to Louis Chang [:louis] from comment #7)
> So one click includes mouse down and mouse up. And click on the first
> checkbox means click on the left most checkbox in the picture (checkbox with
> only grey border), then we will receive the events (1, 2, 3, 5, 10, 12, 22,
> 29, 39, 43, 44) and paint out the second checkbox (checkbox with orange
> border and checkmark). Next, click on the second checkobx then we receive
> the events (3, 5, 10, 12, 22, 29, 44) and paint out the third checkbox.
> Finally, after clicking the fourth checkbox we go back to the first stage
> and repeat.

OK, I think I can guess what the screenshot and the states means now.
FYI, your description above is still rather confusing since there is
only one checkbox (<input> element) in the testcase, so when you say
"first checkbox [...] second checkbox" that implies more than one
checkbox.  What you mean are different renderings of the same checkbox
after a certain state change has occurred as a result of your gestures.
So it would have been clearer if you had described the screenshot as
"the leftmost rendering is after pressing the control (aka mousedown),
the next is after releasing it (mouseup)" etc.

Comment 13

[Mats Palmgren (inactive)]

Attachment: Testcase #3

So, this is the same testcase but with 'opacity' removed on the <input>
so that it's actually rendered, and with a 'outline' added to <input>
and <label> so we can see the extent of those boxes.

Using a mouse to click on Linux, there are two different behaviors depending
on where exactly you click.  You can click directly over the <input>, or on
the <label> (but outside the <input>).

Here's what happens when I click on the <input> in Testcase #3:

Firefox on Linux:
Action:
  press left mouse button (but do not release it) over the checkbox
Result:
  mousedown event on <input> and <label> (see console output)
  focus event on <input>
  orange box-shadow is painted around <i> since the "input:focus+i" selector matches

Action:
  release left mouse button over the checkbox
Result:
  mouseup+click events on <input> and <label>
  <i> gets a checkmark background image since the "input:checked+i" selector matches
  orange box-shadow is still painted around <i> since the <input> still has focus

Action:
  press left mouse button (but do not release it) over the checkbox
Result:
  mousedown events on <input> and <label> (see console output)
  (no change in rendering)

Action:
  release left mouse button over the checkbox
Result:
  mouseup+click events on <input> and <label>
  <i>'s background image disappears since the <input> is no longer checked
  orange box-shadow is still painted around <i> since the <input> still has focus

================================================================================

Here's what happens when I click on the <label> in Testcase #3
(after Shift+Reloading the page first to reset any state):

Firefox on Linux:
Action:
  press left mouse button (but do not release it) over the <label> (outside the checkbox)
Result:
  mousedown event on <label> only
  (no change in rendering)

Action:
  release left mouse button over the <label> (outside the checkbox)
Result:
  mouseup+click events on <label>, focus event on <input>, click event on both <input> and <label>
  <i> gets a checkmark background image since the "input:checked+i" selector matches
  orange box-shadow is painted around <i> since the <input> now has focus

Action:
  press left mouse button (but do not release it) over the <label> (outside the checkbox)
Result:
  mousedown event on <label>, blur event on <input>
  orange box-shadow disappears since the <input> is no longer focused

Action:
  release left mouse button over the <label> (outside the checkbox)
Result:
  mouseup+click events on <label>, focus on <input>, click on <input> and <label>
  <i>'s background image disappears since the <input> is no longer checked
  orange box-shadow is painted around <i> since the <input> now has focus again

================================================================================

Chrome on Linux has exactly the same behavior in both scenarios above.

Comment 14

[Mats Palmgren (inactive)]

So, looking at your screenshot again, it seems it corresponds to the second
scenario, i.e. using a finger to click the "control" hits the <label>.
For that particular case, it matches exactly what is rendered on Linux,
and that behavior/rendering is correct AFAICT.

Comment 15

[Louis Chang [:louis]]

Attachment: fout_states

Comment 16

[Louis Chang [:louis]]

(In reply to Louis Chang [:louis] from comment #15)
> Created attachment 8919557 [details]
> fout_states

Label a number for each rendering of the checkbox.

Comment 17

[Louis Chang [:louis]]

(In reply to Mats Palmgren (:mats) from comment #14)
> So, looking at your screenshot again, it seems it corresponds to the second
> scenario, i.e. using a finger to click the "control" hits the <label>.
> For that particular case, it matches exactly what is rendered on Linux,
> and that behavior/rendering is correct AFAICT.

So the behavior would be correct if I click on the <input>. The problem is happened in the second scenario (click on the <label>). But what happens exactly on Android is when I click (mousedown+mouseup) on the <label> of the leftmost rendering (number 1), it would render number 2. Next, click (mousedown+mouseup) on the <label> of number 2 it would render number 3. Then click on the <label> of number 3, it would render number 4. Last, click on the <label> of number 4, it would render number 5 (the same as number 1). So it is not the same with the rendering on Linux.

By the way, I could not do the action "press left mouse button (but do not release it" on Android. Since if I keep mouse down on <input> or <label> for one second, it will select it and pop up some options (select all, copy, share etc.)... So we will always receive the mousedown and mouseup events together after a click. Anyway, here's the events we receive with testcase#3 when clicking on the <label>:

Action:
  click on the <label> of number 1 (click and release it).
Result:
  mousedown+mouseup events on <label>, focus event on <input>, click event on both <input> and <label>
  rendering is like number 2: <i> gets a checkmark background image since the "input:checked+i" selector matches, orange
  box-shadow is painted around <i> since the <input> now has focus

Action:
  click on the <label> of number 2 (click and release it).
Result:
  mousedown+mouseup events on the <label>, blur event on <input>
  rendering is like number 3: orange box-shadow disappears since the <input> is no longer focused, but no click event on <input>, 
  so the checkmark is still in <input> and so does the background image (with checkmark) in <label>

Action:
  click on the <label> of number 3 (click and release it).
Result:
  mousedown+mouseup events on <label>, focus event on <input>, click event on both <input> and <label>
  rendering is like number 4: <i>'s background image disappears since the <input> is no longer checked, orange
  box-shadow is painted around <i> since the <input> now has focus again

Action:
  click on the <label> of number 4 (click and release it).
Result:
  mousedown+mouseup events on the <label>, blur event on <input>
  rendering is like number 4 (or 1): orange box-shadow disappears since the <input> is no longer focused, and no click event on 
  <input>, so it remains uncheck and no background image on <i>

Maybe there's something to do with the pop up selections that make the events we received messed up?

Comment 18

[Mats Palmgren (inactive)]

> Next, click (mousedown+mouseup) on the <label> of number 2 it would render number 3.

Hmm, does clicking a checked checkbox really NOT uncheck it?
That's clearly a bug that we need to fix.

(Similarly, if it really takes two clicks to go from 3 to 5, that's also
a bug (presumably the same) since clicking on 4 should make it checked.)

I guess this is the source of my confusion above, since it's hard to believe
checkboxes are already this broken on Android.  I assumed that 1->2 was from
the mousedown and 2->3 from the mouseup, which would at least make some sense.

> I could not do the action "press left mouse button (but do not release it" on Android. Since if I keep mouse down on <input> or <label> for one second, it will select it and pop up some options

Oh, OK.  Assuming you click faster so that doesn't happen, I'm pretty sure
there's a separate state change for the "mousedown" action and a separate state
change for the "mousedown" action though, but perhaps we don't need to worry
about any rendering differences between the down/up states for now and just
concentrate on the click bug(s) you describe.

>Action:
>  click on the <label> of number 2 (click and release it).
>Result:
>  mousedown+mouseup events on the <label>, blur event on <input>
>  rendering is like number 3: orange box-shadow disappears since the <input> is no longer focused,
>  but no click event on <input>, 

The last line there is clearly a bug.  We need to figure out why we don't
get a click event for this click gesture; because we should and it should
make the <input> unchecked.

Comment 19

[Louis Chang [:louis]]

(In reply to Mats Palmgren (:mats) from comment #18)
> Hmm, does clicking a checked checkbox really NOT uncheck it?
> That's clearly a bug that we need to fix.

Yes. I'm afraid so.
 
> Oh, OK.  Assuming you click faster so that doesn't happen, I'm pretty sure
> there's a separate state change for the "mousedown" action and a separate
> state
> change for the "mousedown" action though, but perhaps we don't need to worry
> about any rendering differences between the down/up states for now and just
> concentrate on the click bug(s) you describe.

Ya, I suppose it is not the main reason that cause the bug.

> I guess this is the source of my confusion above, since it's hard to believe
> checkboxes are already this broken on Android.  
>
> The last line there is clearly a bug.  We need to figure out why we don't
> get a click event for this click gesture; because we should and it should
> make the <input> unchecked.

Ya... but if we click on the <input> it works normally. And I think most of the cases, checkbox still works normally. Anyway, as I mentioned, if we remove the rule
input:focus+i:after {
  content:' ';
}
The test case could work normally even if we click on the <label>.
So maybe we can start from here and see why this rule causes <input> not getting the click event?

Comment 20

[Mats Palmgren (inactive)]

Yeah, if removing the "input:focus+i:after" rule makes it work when
clicking on the <label> that's a good lead.  I suspect that dynamically
inserting that pseudo-element causes frame construction, perhaps even
on the <label>?  I can see how that might interfere with event delivery. 
It's a bit odd that only Android is affected though, since I'd expect
frame construction / event code to be fairly cross-platform.

Comment 21

[Mats Palmgren (inactive)]

Karl, does this bug affect all Amazon sites or just the .fr one?

Comment 22

[Karl Dubost💡 :karlcow]

Let me check that.

* amazon.com, amazon.co.jp and amazon.fr are affected. Clicking on the labels text is more reliable than the checkbox itself. Reaction time is bizarre with the checkbox.

* amazon.co.in is not but it has a slightly different UI. Clicking on the text this is working as expected. Clicking on the checkbox it doesn't work at all.

Comment 23

[Karl Dubost💡 :karlcow]

Using https://bugzilla.mozilla.org/attachment.cgi?id=8919535
from matt, I get the right behavior currently in Firefox Nightly 68.

But the issue still exists on Amazon website.

Comment 24

[Firefox Bug Husbandry Bot]

Migrating Webcompat whiteboard priorities to project flags. See bug 1547409.

Comment 25

[Firefox Bug Husbandry Bot]

See bug 1547409. Migrating whiteboard priority tags to program flags.

Comment 26

[Thomas Wisniewski [:twisniewski]]

I don't seem to see any web platform tests covering this kind of potential bug, so once it's possible to run them on GeckoView with testdriver.js, it might be worth adding one.

Comment 27

[Chris Peterson [:cpeterson]]

Tom says he'll look into this bug to determine the next steps (but not necessarily execute those next steps yet).

Comment 28

[David Lawrence [:dkl]]

Moving all open Core::Widget: Android bugs to GeckoView::General (then the triage owner of GeckoView will decide which ones are valuable and which ones should be closed).

Comment 29

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:fluffyemily, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 30

[Chris Peterson [:cpeterson]]

Closing as WFM because I can't reproduce this bug in Firefox Nightly 99.