Error when interacting with disabled form controls

[ro0gr]

With this, any action helper(click, triggerEvent,...) would fail on attempt to be invoked against input, textarea, select in disabled state.

I've extracted it from #741, cause fillIn and typeIn seem to be a bit different cases, and it should be easier to review this way. Also this is possibly a breaking change, so we may want to postpone merging it, while fillIn and typeIn seem to be more critical to fix.

[Turbo87]

I'm not sure if we should throw an error for this. A real user is absolutely able to click on a disabled element, the difference though is that nothing will/should happen because the element is disabled.

[ro0gr]

The intention of the PR is to disallow any interactions over disabled form elements. It seems the most effective to achieve it via throwing in the focus. This throw statement in click has been put here just for a better error message. If remove it, it'd fail later in the __focus__ util anyways.

A real user is absolutely able to click on a disabled element

yes, I agree about the user perspective. However, test env seems to be a bit different. If we don't fail as fast as we can, tracking the source of invalid test results can be more difficult then it could be if we were more strict.
In tests, we usually have all the state under control, and before clicking smth, we, as test authors, should likely know a desirable state for the button.

For instance, if we have a button which is responsible for some computation, and we click it while it's disabled(because of some bug), we may have a failing assert for the expected computation value. And since we don't fail fast, the more distance between the action and assert is, the more difficult to track the source of the issue. I believe test helpers could become more helpful here, by letting the user know if he tries to interact w/ something which is not supposed to be interactable.

[ro0gr]

There are 2 possible reasons I can see, why someone clicks on a disabled element:

• they intentionally did want to check if smth doesn't occur after click. In this case, the question is why not to just check for disabled value? And it also encourages the fail-late approach which I've tried to describe in the previous comment.
• the button should not have been disabled, but disabled because of some code issue. In this case we fall again in a "fail late" thing. And there is even a probability to have a false-positive test results if some assert is missing.

Curious if there are any other use cases for clicking disabled elements in tests? 🤔

[btecu]

I agree with @Turbo87 - it shouldn't throw, just do nothing.

[ro0gr]

I'd be happy to agree as well. But don't see a good reason for that. The only one which is raised:

A real user is absolutely able to click on a disabled element, the difference though is that nothing will/should happen because the element is disabled.

however, in addition to my previous arguments, I don't find this aligned with some of the current behaviors. For instance, we currently fail on attempt to focus non-focusable elements

ember-test-helpers/addon-test-support/@ember/test-helpers/dom/focus.ts

Lines 70 to 72 in 978f1b2

	if (!isFocusable(element)) {
	throw new Error(`${target} is not focusable`);
	}

while in reality a user can absolutely try to focus on a disabled element or smth else w/o any success or exceptions in UI.

Or, we fail on attempt to fill-in non-editable elements

ember-test-helpers/addon-test-support/@ember/test-helpers/dom/fill-in.ts

Lines 40 to 42 in 978f1b2

	if (!isControl && !element.isContentEditable) {
	throw new Error('`fillIn` is only usable on form controls or contenteditable elements.');
	}

As far as I see, these all are just examples of fast feedback for the test helpers user, which allows restrict he/she from doing pointless things.

[rwjblue]

In general, I agree with @ro0gr here. The general purpose helpers we provide here are intended to be demonstrating the functionality of the application, allowing click('.some-disabled-thing') to silently do nothing seems almost certainly to be an actual logic / error condition.

[rwjblue]

@ro0gr - Would you mind rebasing (I think the conflicts are likely due to prettier changes)?

[ro0gr]

@rwjblue sure, will do, after we settle #741

[rwjblue]

OK, just rebased now that #741 and #779 have landed.

[amk221]

How can we migrate our tests to support this new behaviour?

Take the following scenario:

await triggerEvent('button[disabled]', 'mouseenter');
assert.dom('.tooltip').hasText("You can't do this because reason")

This used to work, but now it throws. And, even if caught, the mouseenter event never fires.

[ijlee2]

I replied on Discord:

I'm not sure if a mouseenter event should be fired on a disabled button. I wonder if a solution is to create a transparent element when the button is disabled and use CSS to place that element on top of the button. In test, you would mouseenter on that element instead of the button, then check what text you see in the tooltip.

Curious to learn what other approaches can be taken.

[Turbo87]

I'm not sure if a mouseenter event should be fired on a disabled button.

if the browser fires one then we should do it here too

[amk221]

rwjblue pointed out that even though the user can 'interact' with a disabled button, the browser won't fire the mouseenter event anyway. So I think this PR was correct - and my above code snippet is wrong - it just worked incorrectly.

[Turbo87]

I think @rwjblue only pointed out that we should follow what the real browsers are doing, but if you're referring to the Discord conversation, then he didn't say that he knew exactly what the browsers were doing in this case. In other words: this will need to be investigated before taking any decision whether this is correct or not :)

[ro0gr]

Great finding! Ya, this seems to be a bug.

Here I've built a quick playground to found out events, which should be triggered for disabled buttons https://codepen.io/ro0gr/pen/MWjEjRE. Hope I haven't missed any events, just copy pasted all the events known to the triggerEvent.

According to the playground we do currently miss to trigger at least the following events for the disabled elements:

• mouseenter
• mouseleave
• mousemove
• mouseout
• mouseover

Unfortunately, I'm a bit busy with some other activities right now. Will try to find some time to look at it this weekend, if someone else would not do it earlier.

[ro0gr]

oh.. seems like events mentioned above are triggered in FF only on my machine. Chrome doesn't react at all 🤔

[ro0gr]

maybe a "good" solution could be to leave the triggerEvent alone then, and let the user decide if he still wants to trigger some event over a disabled element or not. Since it can be undesirably hard to maintain a proper behavior per each browser...

[rwjblue]

I think logically being disabled means no events (e.g. the behavior added in this PR) and that matches Chrome / Edge / Safari behavior. I don't really want to have divergent by browser here in the helpers, so I think I'd prefer to take a more conservative approach (which this PR does). I do think it would be fine to add an explicit option that could be used to force an event interaction (e.g. to override this limitation), but we should have that discussion separately from this PR thread.

Overall, I think this PR did the right thing and it points out logical issues in folks' test suites (which was the point).

[rwjblue]

Oh, also, I inferred it above but I also tested this in Safari and IE11. IE11 fires events for everything regardless of disabled status, Safari fires no events on the disabled button, and FireFox only fires mousemove events (not click, dblclick, etc).