Skip to content

Conversation

@liangdrime
Copy link
Contributor

On Mac, we’ve run into an issue (#3214) where the page jumps back to the top after adding a column or row using the "+" button in a table.

This seems to be a common WebKit issue—the click event causes the editable area to lose focus, which in turn makes the editor lose its selection.

We noticed there’s already a fix for Safari that caches the selection and restores it when the editor regains focus after onBlur.

// Safari has problem to handle onBlur event. When blur, we cannot get the original selection from editor.
// So we always save a selection whenever editor has focus. Then after blur, we can still use this cached selection.
if (newSelection?.type == 'range') {
   if (this.isSafari) {
      this.state.selection = newSelection;
   }
}

Since iOS and macOS both use WebKit and should behave the same way as Safari, this PR updates the scope of isSafari environment to include both iOS and macOS so they can leverage the same Safari-specific fixes.

@liangdrime
Copy link
Contributor Author

Cc @haven2world for awareness.

@liangdrime
Copy link
Contributor Author

Hi @JiuqingSong, what do you think about this change?

@JiuqingSong
Copy link
Collaborator

Ideally it should be fine. The only concern is this will impact all places where isSafari is used, so please make sure you have gone through all those places and tested they are working well. If you don't have enough confidence about this, you can just add another property there, say isWebkit, and use this check only for your scenario.

@liangdrime
Copy link
Contributor Author

Hi @JiuqingSong, sorry for the late response.

I checked all the places where isSafari is used, and most are related to table and image editing. I tested those scenarios, and everything looks good.

@haven2world also helped review the cases where isSafari is used. We both believe it's fine to update this environment definition, which would allow us to take full advantage of all the Safari fixes on both iOS and Mac.

Please let us know if you have any concerns.

@JiuqingSong JiuqingSong merged commit a5b653f into microsoft:master Dec 8, 2025
1 check passed
JiuqingSong added a commit that referenced this pull request Dec 8, 2025
JiuqingSong added a commit that referenced this pull request Dec 8, 2025
@JiuqingSong
Copy link
Collaborator

@liangdrime I merged then reverted this PR because it breaks some test cases. Please fix test and resubmit the PR. Thanks

(I need to check why it can still pass the check before merge even test case is failed)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants