Skip to content

Commit

Permalink
label with attribute tabIndex should be focused (closes #3501) (#3939)
Browse files Browse the repository at this point in the history
  • Loading branch information
AlexKamaev authored and AndreyBelym committed Jul 1, 2019
1 parent 0a049f6 commit 6c447db
Show file tree
Hide file tree
Showing 13 changed files with 162 additions and 21 deletions.
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -114,7 +114,7 @@
"source-map-support": "^0.5.5",
"strip-bom": "^2.0.0",
"testcafe-browser-tools": "1.6.8",
"testcafe-hammerhead": "14.6.9",
"testcafe-hammerhead": "14.6.10",
"testcafe-legacy-api": "3.1.11",
"testcafe-reporter-json": "^2.1.0",
"testcafe-reporter-list": "^2.1.0",
Expand Down
38 changes: 36 additions & 2 deletions src/client/automation/playback/click/click-command.js
Expand Up @@ -27,6 +27,36 @@ class ElementClickCommand {
}
}

class LabelElementClickCommand extends ElementClickCommand {
constructor (eventState, eventArgs) {
super(eventState, eventArgs);

this.label = this.eventArgs.element;
this.input = getElementBoundToLabel(this.eventArgs.element);
}

run () {
let focusRaised = false;

const ensureFocusRaised = e => {
focusRaised = e.target === this.input;
};

listeners.addInternalEventListener(window, ['focus'], ensureFocusRaised);

super.run();

listeners.removeInternalEventListener(window, ['focus'], ensureFocusRaised);

if (domUtils.isElementFocusable(this.label) && !focusRaised)
this._ensureBoundElementFocusRaised();
}

_ensureBoundElementFocusRaised () {
eventSimulator.focus(this.input);
}
}

class SelectElementClickCommand extends ElementClickCommand {
constructor (eventState, eventArgs) {
super(eventState, eventArgs);
Expand Down Expand Up @@ -63,11 +93,11 @@ class OptionElementClickCommand extends ElementClickCommand {
}
}

class LabelledCheckboxElementClickCommand extends ElementClickCommand {
class LabelledCheckboxElementClickCommand extends LabelElementClickCommand {
constructor (eventState, eventArgs) {
super(eventState, eventArgs);

this.checkbox = getElementBoundToLabel(this.eventArgs.element);
this.checkbox = this.input;
}

run () {
Expand Down Expand Up @@ -98,6 +128,7 @@ export default function (eventState, eventArgs) {
const elementBoundToLabel = getElementBoundToLabel(eventArgs.element);
const isSelectElement = domUtils.isSelectElement(eventArgs.element);
const isOptionElement = domUtils.isOptionElement(eventArgs.element);
const isLabelElement = domUtils.isLabelElement(eventArgs.element) && elementBoundToLabel;
const isLabelledCheckbox = elementBoundToLabel && domUtils.isCheckboxElement(elementBoundToLabel);

if (isSelectElement)
Expand All @@ -109,6 +140,9 @@ export default function (eventState, eventArgs) {
if (isLabelledCheckbox)
return new LabelledCheckboxElementClickCommand(eventState, eventArgs);

if (isLabelElement)
return new LabelElementClickCommand(eventState, eventArgs);

return new ElementClickCommand(eventState, eventArgs);
}

Expand Down
17 changes: 12 additions & 5 deletions src/client/automation/utils/utils.js
Expand Up @@ -43,9 +43,9 @@ export function focusAndSetSelection (element, simulateFocus, caretPos) {
const isTextEditable = domUtils.isTextEditableElement(element);
const labelWithForAttr = domUtils.closest(element, 'label[for]');
const isElementFocusable = domUtils.isElementFocusable(element);
const shouldFocusByRelatedElement = !domUtils.isElementFocusable(element) && labelWithForAttr;
const shouldFocusByRelatedElement = labelWithForAttr;
const isContentEditable = domUtils.isContentEditableElement(element);
let elementForFocus = isContentEditable ? contentEditable.findContentEditableParent(element) : element;
let elementForFocus = isContentEditable ? contentEditable.findContentEditableParent(element) : element;

// NOTE: in WebKit, if selection was never set in an input element, the focus method selects all the
// text in this element. So, we should call select before focus to set the caret to the first symbol.
Expand All @@ -55,15 +55,15 @@ export function focusAndSetSelection (element, simulateFocus, caretPos) {
// NOTE: we should call focus for the element related with a 'label' that has the 'for' attribute
if (shouldFocusByRelatedElement) {
if (simulateFocus)
focusByRelatedElement(labelWithForAttr);
focusByLabel(labelWithForAttr);

resolve();
return;
}

const focusWithSilentMode = !simulateFocus;
const focusForMouseEvent = true;
let preventScrolling = false;
let preventScrolling = false;

if (!isElementFocusable && !isContentEditable) {
const curDocument = domUtils.findDocument(elementForFocus);
Expand Down Expand Up @@ -107,12 +107,19 @@ export function focusAndSetSelection (element, simulateFocus, caretPos) {

export function getElementBoundToLabel (element) {
const labelWithForAttr = domUtils.closest(element, 'label[for]');
const control = labelWithForAttr && labelWithForAttr.control;
const control = labelWithForAttr && (labelWithForAttr.control || document.getElementById(labelWithForAttr.htmlFor));
const isControlVisible = control && styleUtils.isElementVisible(control);

return isControlVisible ? control : null;
}

export function focusByLabel (label) {
if (domUtils.isElementFocusable(label))
focusBlurSandbox.focus(label, testCafeCore.noop, false, true);
else
focusByRelatedElement(label);
}

export function focusByRelatedElement (element) {
const elementForFocus = getElementBoundToLabel(element);

Expand Down
1 change: 1 addition & 0 deletions src/client/core/utils/dom.js
Expand Up @@ -25,6 +25,7 @@ export const isTextAreaElement = hammerhead.utils.dom.isTex
export const isAnchorElement = hammerhead.utils.dom.isAnchorElement;
export const isImgElement = hammerhead.utils.dom.isImgElement;
export const isFormElement = hammerhead.utils.dom.isFormElement;
export const isLabelElement = hammerhead.utils.dom.isLabelElement;
export const isSelectElement = hammerhead.utils.dom.isSelectElement;
export const isRadioButtonElement = hammerhead.utils.dom.isRadioButtonElement;
export const isColorInputElement = hammerhead.utils.dom.isColorInputElement;
Expand Down
4 changes: 2 additions & 2 deletions test/client/before-test.js
Expand Up @@ -102,12 +102,12 @@
// With this hack, we only allow setting the scroll by a script and prevent native browser scrolling.
if (hammerhead.utils.browser.isIOS) {
document.addEventListener('DOMContentLoaded', function () {
const originWindowScrollTo = window.scrollTo;
const originWindowScrollTo = hammerhead.nativeMethods.scrollTo;

let lastScrollTop = window.scrollY;
let lastScrollLeft = window.scrollX;

window.scrollTo = function () {
hammerhead.nativeMethods.scrollTo = function () {
lastScrollLeft = arguments[0];
lastScrollTop = arguments[1];

Expand Down
4 changes: 2 additions & 2 deletions test/client/fixtures/automation/click-test.js
Expand Up @@ -318,7 +318,7 @@ $(document).ready(function () {
backgroundColor: '#ff0000'
});

window.scrollTo(0, 5050);
hammerhead.nativeMethods.scrollTo.call(window, 0, 5050);

const click = new ClickAutomation(target[0], {
offsetX: 10,
Expand Down Expand Up @@ -364,7 +364,7 @@ $(document).ready(function () {
height: 100
});

window.scrollTo(0, 5050);
hammerhead.nativeMethods.scrollTo.call(window, 0, 5050);

const click = new ClickAutomation(target[0], {
offsetX: 10,
Expand Down
Expand Up @@ -48,12 +48,13 @@

// NOTE: Try to avoid odd scrolls in iOS on sauceLabs
if (isIOS) {
const originWindowScrollTo = window.scrollTo;
const nativeMethods = window['%hammerhead%'].nativeMethods;
const originWindowScrollTo = nativeMethods.scrollTo;

let lastScrollTop = window.scrollY;
let lastScrollLeft = window.scrollX;

window.scrollTo = function () {
nativeMethods.scrollTo = function () {
lastScrollLeft = arguments[0];
lastScrollTop = arguments[1];

Expand Down
7 changes: 4 additions & 3 deletions test/functional/fixtures/regression/gh-1353/pages/index.html
Expand Up @@ -30,16 +30,17 @@
// NOTE: scrolling has issues in iOS Simulator https://github.com/DevExpress/testcafe/issues/1237
<script>
const userAgent = window.navigator.userAgent.toLocaleLowerCase();
const isIOS = /(iphone|ipod|ipad)/.test(userAgent);
const isIOS = /(iphone|ipod|ipad)/.test(userAgent);

// NOTE: Try to avoid odd scrolls in iOS on sauceLabs
if (isIOS) {
const originWindowScrollTo = window.scrollTo;
const nativeMethods = window['%hammerhead%'].nativeMethods;
const originWindowScrollTo = nativeMethods.scrollTo;

let lastScrollTop = window.scrollY;
let lastScrollLeft = window.scrollX;

window.scrollTo = function () {
nativeMethods.scrollTo = function () {
lastScrollLeft = arguments[0];
lastScrollTop = arguments[1];

Expand Down
46 changes: 46 additions & 0 deletions test/functional/fixtures/regression/gh-3501/pages/index.html
@@ -0,0 +1,46 @@
<!DOCTYPE html>
<html>
<head>
</head>
<body id="body">
<div>
<label id="label1" for="radio" tabindex="1">label1</label>
<input type="radio" id="radio">

<label id="label2" for="checkbox" tabindex="2">label2</label>
<input type="checkbox" id="checkbox">
</div>
<span id='logger'></span>
<script>
var eventLog = [];

function consoleLog (text) {
var logger = document.querySelector('#logger');

logger.innerHTML += text + '<br/>';
}

var label1 = document.querySelector('#label1');
var label2 = document.querySelector('#label2');
var radio = document.querySelector('#radio');
var checkbox = document.querySelector('#checkbox');

function handleEvent (event) {
const text = event.type + '. Target: ' + event.target.id;

eventLog.push(text);

consoleLog(text);
}

label1.addEventListener('focus', handleEvent);
label2.addEventListener('focus', handleEvent);

label1.addEventListener('click', handleEvent);
label2.addEventListener('click', handleEvent);

radio.addEventListener('focus', handleEvent);
checkbox.addEventListener('focus', handleEvent);
</script>
</body>
</html>
11 changes: 11 additions & 0 deletions test/functional/fixtures/regression/gh-3501/test.js
@@ -0,0 +1,11 @@
describe('[Regression](GH-3501) - Should focus label if it is bound to element and has tabIndex attribute', function () {
it('Click label bound to radio', function () {
return runTests('testcafe-fixtures/index.js', 'Label bound to radio is focused');
});

it('Click label bound to checkbox', function () {
return runTests('testcafe-fixtures/index.js', 'Label bound to checkbox is focused');
});
});


@@ -0,0 +1,38 @@
fixture `Should focus label if it is bound to element and has tabIndex attribute`
.page `http://localhost:3000/fixtures/regression/gh-3501/pages/index.html`;

import { Selector, ClientFunction } from 'testcafe';

const label1 = Selector('#label1');
const label2 = Selector('#label2');

const getLog = ClientFunction(() => {
return window.eventLog;
});

test(`Label bound to radio is focused`, async t => {
await t.click(label1);

const log = await getLog();

await t.expect(log).eql([
'focus. Target: label1',
'click. Target: label1',
'focus. Target: radio'
]);


});

test(`Label bound to checkbox is focused`, async t => {
await t.click(label2);

const log = await getLog();

await t.expect(log).eql([
'focus. Target: label2',
'click. Target: label2',
'focus. Target: checkbox'
]);
});

5 changes: 3 additions & 2 deletions test/functional/fixtures/regression/gh-883/pages/index.html
Expand Up @@ -13,12 +13,13 @@

// NOTE: Try to avoid odd scrolls in iOS on sauceLabs
if (isIOS) {
const originWindowScrollTo = window.scrollTo;
const nativeMethods = window['%hammerhead%'].nativeMethods;
const originWindowScrollTo = nativeMethods.scrollTo;

let lastScrollTop = window.scrollY;
let lastScrollLeft = window.scrollX;

window.scrollTo = function () {
nativeMethods.scrollTo = function () {
lastScrollLeft = arguments[0];
lastScrollTop = arguments[1];

Expand Down
5 changes: 3 additions & 2 deletions test/functional/fixtures/regression/gh-973/pages/index.html
Expand Up @@ -29,12 +29,13 @@

// NOTE: Try to avoid odd scrolls in iOS on sauceLabs
if (isIOS) {
const originWindowScrollTo = window.scrollTo;
const nativeMethods = window['%hammerhead%'].nativeMethods;
const originWindowScrollTo = nativeMethods.scrollTo;

let lastScrollTop = window.scrollY;
let lastScrollLeft = window.scrollX;

window.scrollTo = function () {
nativeMethods.scrollTo = function () {
lastScrollLeft = arguments[0];
lastScrollTop = arguments[1];

Expand Down

0 comments on commit 6c447db

Please sign in to comment.