Prevent chrome from changing checkbox checked state when button or li…

…nk located inside the label is clicked(closes #6949) (#6950) * prevent chrome from changing checkbox checked state when button or link located inside the label is clicked(closes #6949) * code refactor
DevExpress · Mar 30, 2022 · 36c3a8f · 36c3a8f
1 parent b1acfc5
commit 36c3a8f
Show file tree

Hide file tree

Showing 10 changed files with 163 additions and 6 deletions.
diff --git a/src/client/automation/playback/click/click-command.js b/src/client/automation/playback/click/click-command.js
@@ -32,8 +32,8 @@ class LabelElementClickCommand extends ElementClickCommand {
     constructor (eventState, eventArgs) {
         super(eventState, eventArgs);
 
-        this.label = this.eventArgs.element;
-        this.input = getElementBoundToLabel(this.eventArgs.element);
+        this.targetElement = this.eventArgs.element;
+        this.input         = getElementBoundToLabel(this.eventArgs.element);
     }
 
     run () {
@@ -49,7 +49,7 @@ class LabelElementClickCommand extends ElementClickCommand {
 
         listeners.removeInternalEventBeforeListener(window, ['focus'], ensureFocusRaised);
 
-        if (domUtils.isElementFocusable(this.label) && !focusRaised)
+        if (domUtils.isElementFocusable(this.targetElement) && !focusRaised)
             this._ensureBoundElementFocusRaised();
     }
 
@@ -114,7 +114,10 @@ class LabelledCheckboxElementClickCommand extends LabelElementClickCommand {
 
         listeners.removeInternalEventBeforeListener(window, ['change'], onChange);
 
-        if (browserUtils.isChrome && !changed)
+        // NOTE: Two overlapping issues: https://github.com/DevExpress/testcafe/issues/3348 and https://github.com/DevExpress/testcafe/issues/6949
+        // When label contains <a href=any> or <button> element, clicking these elements will prevent checkbox from changing checked state.
+        // We should to leave the code for fixing .focus issue and add additional check for the clickable elements inside the label:
+        if (browserUtils.isChrome && !changed && !this._isClickableElementInsideLabel(this.targetElement))
             this._ensureCheckboxStateChanged();
     }
 
@@ -123,6 +126,13 @@ class LabelledCheckboxElementClickCommand extends LabelElementClickCommand {
 
         eventSimulator.change(this.checkbox);
     }
+
+    _isClickableElementInsideLabel (element) {
+        const isClickableLink = domUtils.isAnchorElement(element) && element.getAttribute('href');
+        const isButton        = domUtils.isButtonElement(element);
+
+        return isClickableLink || isButton;
+    }
 }
 
 export default function (eventState, eventArgs) {
@@ -146,5 +156,3 @@ export default function (eventState, eventArgs) {
 
     return new ElementClickCommand(eventState, eventArgs);
 }
-
-
diff --git a/test/functional/fixtures/regression/gh-6949/pages/with-button.html b/test/functional/fixtures/regression/gh-6949/pages/with-button.html
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+    <meta charset="UTF-8">
+    <title>Title</title>
+</head>
+<body>
+<div class="checkbox">
+    <input id="checkbox" type="checkbox">
+    <label for="checkbox" id="checkbox-label">
+        I have read and accept
+        <button id="clickable-element" type="button">terms and conditions.</button>
+    </label>
+</div>
+</body>
+</html>
diff --git a/test/functional/fixtures/regression/gh-6949/pages/with-div.html b/test/functional/fixtures/regression/gh-6949/pages/with-div.html
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+    <meta charset="UTF-8">
+    <title>Title</title>
+</head>
+<body>
+<div class="checkbox">
+    <input id="checkbox" type="checkbox">
+    <label for="checkbox" id="checkbox-label">
+        I have read and accept
+        <div id="clickable-element" style="background-color: #00FF00; width: 200px; height: 50px;">
+            terms and conditions.
+        </div>
+    </label>
+</div>
+</body>
+</html>
diff --git a/test/functional/fixtures/regression/gh-6949/pages/with-link-without-href.html b/test/functional/fixtures/regression/gh-6949/pages/with-link-without-href.html
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+    <meta charset="UTF-8">
+    <title>Title</title>
+</head>
+<body>
+<div class="checkbox">
+    <input id="checkbox" type="checkbox">
+    <label for="checkbox" id="checkbox-label">
+        I have read and accept
+        <a id="clickable-element">
+            terms and conditions.
+        </a>
+    </label>
+</div>
+</body>
+</html>
diff --git a/test/functional/fixtures/regression/gh-6949/pages/with-link.html b/test/functional/fixtures/regression/gh-6949/pages/with-link.html
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+    <meta charset="UTF-8">
+    <title>Title</title>
+</head>
+<body>
+<div class="checkbox">
+    <input id="checkbox" type="checkbox">
+    <label for="checkbox" id="checkbox-label">
+        I have read and accept
+        <a id="clickable-element" href="#">
+            terms and conditions.
+        </a>
+    </label>
+</div>
+</body>
+</html>
diff --git a/test/functional/fixtures/regression/gh-6949/test.js b/test/functional/fixtures/regression/gh-6949/test.js
@@ -0,0 +1,23 @@
+describe('[Regression](GH-6949)', () => {
+    it('Should change checkbox state when clicking checkbox label', () => {
+        return runTests('./testcafe-fixtures/with-link.js', 'Click checkbox label');
+    });
+
+    it('Should NOT change checkbox state when clicking a LINK inside the checkbox label', () => {
+        return runTests('./testcafe-fixtures/with-link.js', 'Click link inside checkbox label');
+    });
+
+    it('Should change checkbox state when clicking a LINK without href attribute inside the checkbox label', () => {
+        //The behaviour in IE is different, so we should to exclude it from this test:
+        return runTests('./testcafe-fixtures/with-link-without-href.js', 'Click link without href inside checkbox label', { skip: 'ie' });
+    });
+
+    it('Should NOT change checkbox state when clicking a BUTTON inside the checkbox label', () => {
+        //The behaviour in IE is different, so we should to exclude it from this test:
+        return runTests('./testcafe-fixtures/with-button.js', 'Click button inside checkbox label', { skip: 'ie' });
+    });
+
+    it('Should change checkbox state when clicking a DIV with onclick handler inside the checkbox label', () => {
+        return runTests('./testcafe-fixtures/with-div.js', 'Click div inside checkbox label');
+    });
+});
diff --git a/test/functional/fixtures/regression/gh-6949/testcafe-fixtures/with-button.js b/test/functional/fixtures/regression/gh-6949/testcafe-fixtures/with-button.js
@@ -0,0 +1,12 @@
+import { Selector } from 'testcafe';
+
+fixture`Getting Started`
+    .page`http://localhost:3000/fixtures/regression/gh-6949/pages/with-button.html`;
+
+test('Click button inside checkbox label', async t => {
+    const button = Selector('#clickable-element');
+    const checkBox = Selector('#checkbox');
+
+    await t.click(button);
+    await t.expect(checkBox.checked).eql(false);
+});
diff --git a/test/functional/fixtures/regression/gh-6949/testcafe-fixtures/with-div.js b/test/functional/fixtures/regression/gh-6949/testcafe-fixtures/with-div.js
@@ -0,0 +1,12 @@
+import { Selector } from 'testcafe';
+
+fixture`Getting Started`
+    .page`http://localhost:3000/fixtures/regression/gh-6949/pages/with-div.html`;
+
+test('Click div inside checkbox label', async t => {
+    const div = Selector('#clickable-element');
+    const checkBox = Selector('#checkbox');
+
+    await t.click(div);
+    await t.expect(checkBox.checked).eql(true);
+});
diff --git a/test/functional/fixtures/regression/gh-6949/testcafe-fixtures/with-link-without-href.js b/test/functional/fixtures/regression/gh-6949/testcafe-fixtures/with-link-without-href.js
@@ -0,0 +1,12 @@
+import { Selector } from 'testcafe';
+
+fixture`Getting Started`
+    .page`http://localhost:3000/fixtures/regression/gh-6949/pages/with-link-without-href.html`;
+
+test('Click link without href inside checkbox label', async t => {
+    const link = Selector('#clickable-element');
+    const checkBox = Selector('#checkbox');
+
+    await t.click(link);
+    await t.expect(checkBox.checked).eql(true);
+});
diff --git a/test/functional/fixtures/regression/gh-6949/testcafe-fixtures/with-link.js b/test/functional/fixtures/regression/gh-6949/testcafe-fixtures/with-link.js
@@ -0,0 +1,20 @@
+import { Selector } from 'testcafe';
+
+fixture`Getting Started`
+    .page`http://localhost:3000/fixtures/regression/gh-6949/pages/with-link.html`;
+
+test('Click checkbox label', async t => {
+    const label = Selector('#checkbox-label');
+    const checkBox = Selector('#checkbox');
+
+    await t.click(label);
+    await t.expect(checkBox.checked).eql(true);
+});
+
+test('Click link inside checkbox label', async t => {
+    const link = Selector('#clickable-element');
+    const checkBox = Selector('#checkbox');
+
+    await t.click(link);
+    await t.expect(checkBox.checked).eql(false);
+});