Label with user-select:none acts like a normal text when it comes to preventing events on checkbox

NEW
Unassigned

Status

()

Core
Layout: Form Controls
P4
minor
2 years ago
2 years ago

People

(Reporter: arni2033, Unassigned)

Tracking

({regression, testcase})

1.9.1 Branch
regression, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [parity-Chrome], URL)

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
>>>   My Info:   Win7_64, Nightly 49, 32bit, ID 20160526082509
STR:
1. Open the following "data:" url or click URL in the form above
> data:text/html,<input><label for="first"><input id="first" type="checkbox">First</label><label for="second"><input id="second" type="checkbox">Second</label><label for="third"><input id="third" type="checkbox">third</label><label>abcd</label><style>label{-moz-user-select:-moz-none;-webkit-user-select:none;} *>:focus{outline:1px dashed red}
2. Move mouse between "o" and "n" on the page, hold left mouse button,
   move mouse between "e" and "c" on the page, release left mouse button
3. Doubleclick on the letter "e"

AR:  After Step 2 nothing happens on the page. After Step 3 checkbox isn't focused
ER:  After Step 2 checkbox should be checked. After Step 3 checkbox should be still focused

It caused regression in devtools options as they switched from XUL checkboxes to HTML in bug 1246514.
NI for Olli: please rate this behavior in 2 catgories - (a) good/bad  (b) follows the spec or not.
1) "click and drag" doesn't work on <label>
2) checkbox looses focus after doubleclick on <label>

This is regression from bug 382369. Regression range (between 2009-03-23 and 2009-03-24):
> https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d74e551f46a3&tochange=6cfe70091cfc
Flags: needinfo?(bugs)

Comment 1

2 years ago
mats might have an opinion on this one.

I don't know why after 2 checkbox should be checked.
checkbox not being focused after 3 is a bit weird, so that at least should be consistent with clicking just once.
Flags: needinfo?(bugs) → needinfo?(mats)
(Reporter)

Comment 2

2 years ago
(In reply to Olli Pettay [:smaug] from comment #1)
> I don't know why after 2 checkbox should be checked.
There're several reasons (some may be questionable). Here's full explanation why I have reported this:

1) Spec(?)
I most likely read it wrong, but [1] -> [2] -> [3] seems the right chain for comment 0.
Actually [3] isn't necessary, because 'click' event fires on <label> - that's for sure. Therefore
checkbox should be clicked. The only excuse not to click checkbox is the presence of text selection
- that's per your comment (bug 1037941 comment 2). But there's no text selection in STR in comment 0.
[1] https://html.spec.whatwg.org/#the-label-element
[2] https://html.spec.whatwg.org/#activation
[3] https://w3c.github.io/uievents/#event-type-click

2) Real intention of bug 382369
Open http://mxr.mozilla.org/mozilla-central/source/dom/html/HTMLLabelElement.cpp#158
and scroll that line to the middle of the window (to read text below and above).
Text above checks for selection by looking at mouse coordinate(sic), but the most important part is:
> 156           // Don't click the for-content if we did drag-select text or if we
> 157           // have a kbd modifier (which adjusts a selection).
As you may see, the person who made that patch says "drag-select" and "selection", so he didn't really expect that we're not selecting text at the moment.
The patch is called "Distinguish click from click-drag on a <label> so text selection works".
Therefore original intent of bug382369 was to fix text selection. Not to touch "user-select:none" case

3) GoogleChrome works this way
Chrome works as expected on STR in comment 0. If I remove "user-select:none", it behaves like Firefox,
i.e. allows to select text without clicking checkbox. So web devs will mostly expect this behavior.

4) UX
It's too easy to accidentally move mouse pointer by ~2px while clicking, (especially) on touchpad or if I'm looking at something else on the page, thinking whether I'm doing the right thing or not. When <checkbox>es in devtools options switched to HTML, I noticed it right away, as clicking became harder.


> checkbox not being focused after 3 is a bit weird, so that at least should
> be consistent with clicking just once.
Please use this bug for any sensible valid bug mentioned in comment 0 (if there're no duplicates).
It appears we fire a click event on the <label>, but not on the associated
checkbox.  I think we should probably not fire a click event at all since
the user is actually doing a drag gesture.  We don't fire click when
dragging on an <input type=button> and I think a "user-select:none" label
should probably work the same.  I would be surprised if any spec says
that we should fire a click for a drag gesture.

> It's too easy to accidentally move mouse pointer by ~2px while clicking

So isn't this a problem for ordinary buttons too then?

If the threshold distance to initiate a drag operation is too small on
some platforms then we should probably increase it on those platforms
to fix the problem more generally.

> checkbox looses focus after doubleclick on <label>

That seems like a (minor) bug.  It might be hard to fix and it's a very
rare edge case, so probably not worth spending our limited resources on.
Flags: needinfo?(mats)
(Reporter)

Comment 4

2 years ago
Created attachment 8757711 [details]
bug 1276126 comment 4.webm

(In reply to Mats Palmgren (:mats) from comment #3)
> We don't fire click when dragging on an <input type=button>
That's probably a typo. Is it? You do fire click when dragging on an <input type=button> , see url [1]

> <input type=button> and I think a "user-select:none" label should probably work the same
Nice! Then make "user-select:none" label work the same as <input type=button>

> I would be surprised if any spec says that we should fire a click for a drag gesture.
It works this way on every element. This is either a big typo, or a huge mistake in implementing behavior described above (not firing click for "drag gesture")

> That seems like a (minor) bug. It might be hard to fix and it's a very
> rare edge case, so probably not worth spending our limited resources on.
And as always, bug 382369 is itself a minor bug, but you still chose to "fix" it breaking usability (see 3 open dependent bugs).

[1]   data:text/html,<input type="button" value="asdf" onclick="console.log('click')" onmousedown="console.log('down')" onmouseup="console.log('up')"></input>
Flags: needinfo?(mats)
> That's probably a typo. Is it?

You're right, I must have made a mistake when I tested the button behavior.

So I agree that step 2 in the STR should generate a normal click event
so that a "user-select:none" <label> behaves the same as a button for
that gesture.
Severity: normal → minor
Flags: needinfo?(mats)
Keywords: testcase
Priority: -- → P4
Comment hidden (spam)
> The bug is (in my view), that click event isn't passed to inner <input>

That's what I meant when I said "generate a normal click event" -  the click
event should be forwarded to its target.
Priority: -- → P4
You need to log in before you can comment on or make changes to this bug.