Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

useDisabled hook: Fix problem with not working in iframe #47459

Merged
merged 1 commit into from
Jan 26, 2023

Conversation

t-hamano
Copy link
Contributor

@t-hamano t-hamano commented Jan 26, 2023

Fixes: #47293
More fundamental solution than #47331

What?

This PR solves the problem of the useDisabled hook not working on iframes and ensures that the inert attribute is properly applied to the child element.

As a result, the issue reported in #47293, where tools are duplicated, will be fixed.

Why?

The useDisabled hook grants the inert attribute if its child element is a instance of HTMLElement. However, as far as I know, instanceof HTMLElement is implicitly treated as a window.HTMLElement, so it does not meet this condition in an iframe.
As a result, within the iframe, this hook does not grant the inert attribute.

As you can see in the video below, in the classic theme, the post editor is not an iframe, so the inert attribute is given and the issue reported in #47293 does not occur. However, in the case of the block theme, because it is an iframe, the inert attribute is not given and the problem occurs:

552200a7a9b86c2d5a5fac06b4653bfe.mp4

How?

Refer to defaultView to check instances relative to the defaultView.

More fundamentally, as pointed out in this comment, the problem might be that the clientId is shared within the hoge block.
This issue may need to be investigated separately.

Testing Instructions

  • Activate Thwenty Twenty Three theme.
  • Open the site editor.
  • Open the Home template containing the query loop block.
  • Click on various locations in the query loop to confirm that the problem reported in Randomly duplicating design tools #47293 does not occur.
  • Paste this query loop into the content of the post editor.
  • Similarly, confirm that no problems occur.

@t-hamano t-hamano self-assigned this Jan 26, 2023
@t-hamano t-hamano added [Type] Bug An existing feature does not function as intended [Package] Compose /packages/compose labels Jan 26, 2023
@t-hamano t-hamano marked this pull request as ready for review January 26, 2023 13:46
@t-hamano t-hamano requested a review from ajitbohra as a code owner January 26, 2023 13:46
@t-hamano t-hamano requested review from ntsekouras and youknowriad and removed request for ajitbohra January 26, 2023 13:47
@@ -39,11 +39,16 @@ export default function useDisabled( {
return;
}

const defaultView = node?.ownerDocument?.defaultView;
Copy link
Contributor

@youknowriad youknowriad Jan 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this ever be falsy?

Copy link
Contributor Author

@t-hamano t-hamano Jan 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know in practical cases, but TypeScript will output errors if this check is not present.

falsy

@t-hamano t-hamano merged commit 360e5a4 into trunk Jan 26, 2023
@t-hamano t-hamano deleted the fix/inert-iframe-element branch January 26, 2023 14:03
@github-actions github-actions bot added this to the Gutenberg 15.1 milestone Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Compose /packages/compose [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Randomly duplicating design tools
2 participants