-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Rich text: copy tag name on internal paste #48254
Conversation
Size Change: +3.34 kB (0%) Total Size: 1.44 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem to fix the issue for me on Chrome Version 110.0.5481.100
CleanShot.2023-02-20.at.07.29.30.mp4
Also, it's odd that the bug only appeared recently but is present in older versions of WordPress. Is it possible some browser API changed?
Flaky tests detected in b4b681d95cce3ec478ad077b064d863df3ce71e3. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5658725076
|
1a8fafd
to
d0689c4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pull request is working for me now in Chrome Version 110.0.5481.100 on Mac:
CleanShot.2023-02-21.at.05.26.04.mp4
The scenario still fails on trunk
, so it seems like this is a fix (and not that my browser changed in some way).
I only started experiencing this bug recently. Any ideas what might've changed?
I'm not sure. I think this behaviour has been around for a while |
d0689c4
to
30da760
Compare
Hi team, just following up on the progress here following a discussion ahead of RC1. If we can confirm that this issue is caused by a browser change and can get a fix in place by RC2 (next week), this PR can get included in 6.2. Otherwise, we will need to punt. Thanks! |
According to the issue, it's been around for a long time so it's probably fine to punt? |
Hi @ellatrix, this PR is on the 6.3 project board since it was punted from 6.2. But it's been a while since this was worked on. Is this still something we are aiming to get into 6.3, or should I remove it from the board? |
I keep seeing this so I'm going to punt it :) |
Sorry, got distracted with other work, I'll have a look into. |
30da760
to
b4b681d
Compare
Should be fixed now. Not sure how it was even working before. |
b4b681d
to
09948a7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Working correctly now:
Screen.Recording.2023-07-26.at.11.50.18.AM.mov
@danielbachhuber |
@apeatling Something about my local machine seems fubar, unfortunately. |
I think this one looks good to merge either way. 👍 |
I'll address the e2e test failures on Monday |
150c1cd
to
802fe10
Compare
802fe10
to
09c707c
Compare
Thanks for driving this over the finish line, @ellatrix ! 🙌 |
What?
After copying a heading that is entirely or partially selected, pasting it into a paragraph doesn't copy over the heading tag name.
Why?
Fixes #48040.
How?
When setting the html content on paste, copy the tagName context. On paste this can then be used to create a blocks array.
This PR also changes internal paste to go through the
pasteHandler
path for better consistency in paste handling.Testing Instructions
Select all text in a heading and copy. Paste it in an empty paragraph. The heading should be preserved.
Testing Instructions for Keyboard
Screenshots or screencast