Skip to content

Block Editor: Remove block selection clearer handling from iframe #73753

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.

Sign up for GitHub

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

Merged
Mamaduka merged 1 commit into from
Dec 17, 2025
Merged

Block Editor: Remove block selection clearer handling from iframe #73753

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions packages/block-editor/src/components/iframe/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import { useSelect } from '@wordpress/data';
/**
* Internal dependencies
*/
import { useBlockSelectionClearer } from '../block-selection-clearer';
import { useWritingFlow } from '../writing-flow';
import { getCompatibilityStyles } from './get-compatibility-styles';
import { useScaleCanvas } from './use-scale-canvas';
Expand Down Expand Up @@ -119,7 +118,6 @@ function Iframe( {
/** @type {[Document, import('react').Dispatch<Document>]} */
const [ iframeDocument, setIframeDocument ] = useState();
const [ bodyClasses, setBodyClasses ] = useState( [] );
const clearerRef = useBlockSelectionClearer();
const [ before, writingFlowRef, after ] = useWritingFlow();

const setRef = useRefEffect( ( node ) => {
Expand Down Expand Up @@ -176,8 +174,6 @@ function Iframe( {

documentElement.classList.add( 'block-editor-iframe__html' );

clearerRef( documentElement );

contentDocument.dir = ownerDocument.dir;

for ( const compatStyle of getCompatibilityStyles() ) {
Expand Down Expand Up @@ -243,7 +239,6 @@ function Iframe( {
const bodyRef = useMergeRefs( [
useBubbleEvents( iframeDocument ),
contentRef,
clearerRef,
Copy link
Member

@ellatrix ellatrix Dec 5, 2025

Choose a reason for hiding this comment

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

This probably doesn't make a difference? Not sure why it's both added here and ExperimentalBlockCanvas. Maybe we should keep one, not sure why it was ever added to ExperimentalBlockCanvas.

Copy link
Member Author

@Mamaduka Mamaduka Dec 5, 2025

Choose a reason for hiding this comment

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

What do you mean by difference? The current removal fixes the mentioned.

I thought one in ExperimentalBlockCanvas was responsible for both iframed and non-iframed editors.

Copy link
Member

@ellatrix ellatrix Dec 9, 2025

Choose a reason for hiding this comment

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

I'm curious why it fixes the issue if it's still added by ExperimentalBlockCanvas?

Copy link
Member Author

@Mamaduka Mamaduka Dec 9, 2025

Choose a reason for hiding this comment

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

The clearerRef( documentElement ); attaches events to the iframe document, which then react to clicks on the scrollbar.

The one from ExperimentalBlockCanvas is attached to the body and doesn't have the same problem. This was introduced as part of general framework improvements (#54209).

In my tests, removing the ref callback from this component fixes the bug without introducing any regressions.

Copy link
Member Author

@Mamaduka Mamaduka Dec 11, 2025

Choose a reason for hiding this comment

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

@ellatrix, let me know if there's a different path/solution you want me to pursue.

P.S. I tried deleting the click the scrollbar, but, to my understanding, it's almost impossible to do correctly across browsers and OS.

Copy link
Member

@ellatrix ellatrix Dec 16, 2025

Choose a reason for hiding this comment

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

ExperimentalBlockCanvas adds the clearer as the contentRef, which is added to the same element through the line above this one. I don't get what the difference is? Is there a bug because it is added twice to the same element?

Copy link
Member

@ellatrix ellatrix Dec 16, 2025

Choose a reason for hiding this comment

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

Ah, this one is also added to the body. So why remove it?

Copy link
Member Author

@Mamaduka Mamaduka Dec 17, 2025

Choose a reason for hiding this comment

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

I believe the problem is that the removed ref callback was attached to the html, which was causing the bug.

As far as I can tell, this doesn't introduce any regressions, so let's merge it.

writingFlowRef,
disabledRef,
] );
Expand Down
Loading