code refactor

Artem-Babich · Mar 29, 2022 · 5e06f92 · 5e06f92
1 parent b4b9504
commit 5e06f92
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 11 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();
     }
 
@@ -98,8 +98,7 @@ class LabelledCheckboxElementClickCommand extends LabelElementClickCommand {
     constructor (eventState, eventArgs) {
         super(eventState, eventArgs);
 
-        this.checkbox                           = this.input;
-        this.shouldPreventCheckedChangeInChrome = this._isClickableElementInsideLabel(eventArgs.element);
+        this.checkbox = this.input;
     }
 
     run () {
@@ -115,10 +114,10 @@ class LabelledCheckboxElementClickCommand extends LabelElementClickCommand {
 
         listeners.removeInternalEventBeforeListener(window, ['change'], onChange);
 
-        //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.shouldPreventCheckedChangeInChrome)
+        // 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();
     }
 
@@ -129,8 +128,8 @@ class LabelledCheckboxElementClickCommand extends LabelElementClickCommand {
     }
 
     _isClickableElementInsideLabel (element) {
-        const isClickableLink = element.tagName === 'A' && element.getAttribute('href');
-        const isButton        = element.tagName === 'BUTTON';
+        const isClickableLink = domUtils.isAnchorElement(element) && element.getAttribute('href');
+        const isButton        = domUtils.isButtonElement(element);
 
         return isClickableLink || isButton;
     }

diff --git a/test/functional/fixtures/regression/gh-6949/test.js b/test/functional/fixtures/regression/gh-6949/test.js
@@ -2,17 +2,21 @@ 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');
     });