Skip to content

Commit

Permalink
eslint: Enable @wordpress/no-global-active-element and `@wordpress/…
Browse files Browse the repository at this point in the history
…no-global-get-selection`

These warn about using `document.activeElement` or
`window.getSelection()`, which can do the wrong thing if iframes are
involved (e.g. in the block editor). The correct thing to do is to
access `.ownerDocument` on some target element.

Note that `window.getSelection()` is equivalent to
`window.document.getSelection()`, so I didn't bother doing
`.ownerDocument.defaultView.getSelection()`.
  • Loading branch information
anomiex committed Sep 11, 2024
1 parent 23f7e3c commit 90084c6
Show file tree
Hide file tree
Showing 20 changed files with 59 additions and 19 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

Logo generator: Get selection from the prompt's document rather than the global `window`.
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export const Prompt: React.FC< { initialPrompt?: string } > = ( { initialPrompt
const onPromptPaste = ( event: React.ClipboardEvent< HTMLInputElement > ) => {
event.preventDefault();

const selection = window.getSelection();
const selection = event.currentTarget.ownerDocument.getSelection();
if ( ! selection || ! selection.rangeCount ) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

SSO tooltip: Use anchor element's document instead of the global `document`.
4 changes: 2 additions & 2 deletions projects/packages/connection/src/sso/jetpack-sso-users.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ document.addEventListener( 'DOMContentLoaded', function () {
*/
function removeTooltip() {
// Only remove tooltip if the element isn't currently active.
if ( document.activeElement === tooltip ) {
if ( tooltip.ownerDocument.activeElement === tooltip ) {
return;
}
tooltip.removeChild( tooltipTextbox );
Expand Down Expand Up @@ -56,7 +56,7 @@ document.addEventListener( 'DOMContentLoaded', function () {
* @param {Event} event - Triggering event.
*/
function removeSSOInvitationTooltip( event ) {
if ( document.activeElement === event.target ) {
if ( event.target.ownerDocument.activeElement === event.target ) {
return;
}
this.querySelector( '.jetpack-sso-invitation-tooltip' ).style.display = 'none';
Expand Down
4 changes: 4 additions & 0 deletions projects/packages/forms/changelog/update-more-eslint-cleanup
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

Options: Get selection from the element's document instead of the global `window`.
7 changes: 4 additions & 3 deletions projects/packages/forms/src/blocks/contact-form/util/caret.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,18 @@ export const getCaretPosition = target => {
* @param {HTMLElement} target - Contenteditable element of which to move the caret
*/
export const moveCaretToEnd = target => {
if ( 'undefined' === typeof window ) {
const doc = target.ownerDocument;
if ( 'undefined' === typeof doc ) {
return;
}

// Add the contenteditable element to a new selection and collapse it to the end
const range = document.createRange();
const range = doc.createRange();
range.selectNodeContents( target );
range.collapse( false );

// Clear the window selection object and add the new selection
const selection = window.getSelection();
const selection = doc.getSelection();
selection.removeAllRanges();
selection.addRange( range );
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

Get active element from target's document rather than the global `document`.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ const useFocusHandler = ( ref: React.MutableRefObject< null | HTMLElement > ): b
const [ hasFocus, setHasFocus ] = useState( false );

const handleFocus = useCallback( () => {
if ( document.hasFocus() && ref.current?.contains( document.activeElement ) ) {
const doc = ref.current?.ownerDocument;
if ( doc && doc.hasFocus() && ref.current?.contains( doc.activeElement ) ) {
setHasFocus( true );
} else {
setHasFocus( false );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,17 @@ const useFocusTrap = ( ref: React.MutableRefObject< null | HTMLElement > ): void
if ( event.key === 'Tab' ) {
if ( event.shiftKey ) {
// Shift + Tab
if ( document.activeElement === firstFocusableElement ) {
if (
firstFocusableElement &&
firstFocusableElement.ownerDocument.activeElement === firstFocusableElement
) {
lastFocusableElement?.focus();
handled = true;
}
} else if ( document.activeElement === lastFocusableElement ) {
} else if (
lastFocusableElement &&
lastFocusableElement.ownerDocument.activeElement === lastFocusableElement
) {
// Tab
firstFocusableElement?.focus();
handled = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,10 @@ export const InfoTooltip: FC< Props > = ( {
const hideTooltip = useCallback( () => {
// Don't hide the tooltip here if it's the tooltip button that was clicked (the button
// becoming the document's activeElement). Instead let toggleTooltip() handle the closing.
if ( useTooltipRef.current && ! useTooltipRef.current.contains( document.activeElement ) ) {
if (
useTooltipRef.current &&
! useTooltipRef.current.contains( useTooltipRef.current.ownerDocument.activeElement )
) {
setIsPopoverVisible( false );
}
}, [ setIsPopoverVisible, useTooltipRef ] );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,10 @@ function ThreatStatus( {
const hideTooltip = useCallback( () => {
// Don't hide the tooltip here if it's the tooltip button that was clicked (the button
// becoming the document's activeElement). Instead let toggleTooltip() handle the closing.
if ( useTooltipRef.current && ! useTooltipRef.current.contains( document.activeElement ) ) {
if (
useTooltipRef.current &&
! useTooltipRef.current.contains( useTooltipRef.current.ownerDocument.activeElement )
) {
setIsPopoverVisible( false );
}
}, [ setIsPopoverVisible, useTooltipRef ] );
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

Get active element from tooltip button's document rather than the global `document`.
4 changes: 4 additions & 0 deletions projects/packages/search/changelog/update-more-eslint-cleanup
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

Instant search: Use triggering element's document instead of the global `document`.
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import './search-box.scss';

let initiallyFocusedElement = null;
const stealFocusWithInput = inputElement => () => {
initiallyFocusedElement = document.activeElement;
initiallyFocusedElement = inputElement.ownerDocument.activeElement;
inputElement.focus();
};
const restoreFocus = () => initiallyFocusedElement && initiallyFocusedElement.focus();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,8 @@ class PopoverMenu extends React.Component {
_onShow = () => {
const elementToFocus = this.menuRef.current;

this._previouslyFocusedElement = document.activeElement;

if ( elementToFocus ) {
this._previouslyFocusedElement = elementToFocus.ownerDocument.activeElement;
elementToFocus.focus();
}
};
Expand Down
4 changes: 4 additions & 0 deletions projects/plugins/jetpack/changelog/update-more-eslint-cleanup
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: other

Blocks: Get active element or selection from target element's document instead of the global `document`.
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ export default function AIAssistantEdit( { attributes, setAttributes, clientId,
const acceptLabel = isGeneratingTitle ? acceptTitleLabel : acceptContentLabel;

const moveCaretToEnd = element => {
const selection = window.getSelection();
const selection = element.ownerDocument.getSelection();
selection.selectAllChildren( element );
selection.collapseToEnd();
element.focus();
Expand Down
2 changes: 1 addition & 1 deletion projects/plugins/jetpack/extensions/blocks/map/controls.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ export default ( {
setAttributes( { mapHeight: event.target.value } );
// If this input isn't focussed, the onBlur handler won't be triggered
// to commit the map size, so we need to check for that.
if ( event.target !== document.activeElement ) {
if ( event.target !== event.target.ownerDocument.activeElement ) {
if ( mapRef.current ) {
setTimeout( mapRef.current.sizeMap, 0 );
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ class GalleryImageEdit extends Component {

onImageKeyDown = event => {
if (
this.img.current === document.activeElement &&
this.img.current &&
this.img.current === this.img.current.ownerDocument.activeElement &&
this.props.isSelected &&
[ BACKSPACE, DELETE ].includes( event.keyCode )
) {
Expand Down
2 changes: 0 additions & 2 deletions tools/js-tools/eslintrc/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,5 @@ module.exports = {
'jsx-a11y/label-has-associated-control': [ 'error', { assert: 'either' } ],
'object-shorthand': 'off',
'@wordpress/no-base-control-with-label-without-id': 'off',
'@wordpress/no-global-active-element': 'off',
'@wordpress/no-global-get-selection': 'off',
},
};

0 comments on commit 90084c6

Please sign in to comment.