Iframed editor: Fix relative wp-content URLs #66751
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Soean
added
[Type] Bug
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @tambourine-man, @Pat-Relentless, @nicolasleroy. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +83 B (0%) Total Size: 1.82 MB
ℹ️ View Unchanged
|
youknowriad
reviewed
Nov 5, 2024
| @@ -272,6 +272,7 @@ function Iframe( { | |||
| <html> | |||
| <head> | |||
| <meta charset="utf-8"> | |||
| <base href="${ window.location.origin }"> | |||
There was a problem hiding this comment.
Does this work, regardless of how the site is configured (top level, sub folder, ...) and also for third-party block editors?
There was a problem hiding this comment.
This shouldn't matter for relative urls starting with /, they should always use the root.
But for relative urls that don't start with a slash it's trickier. We'd have always use the current post URL etc. This wouldn't work without an iframe either (in the admin), so not sure if it's worth fixing.
|
Btw I just tested this manually setting an image block's URL. |
getdave
removed
the
Backport to WP 6.7 Beta/RC
|
I trust you'll understand that I've removed the This is because we are now past the final public RC and any further changes targeting the main 6.7 release need to be discussed and approved by a release lead. Looking at the PR it's unclear to me whether this is a bug introduced during the Beta/RC period or something that's been present for a while. If it's a longstanding bug then in the interests of stability I'd vote to defer this to 6.7.1. Update: I noticed the Issue refers to WordPress 6.3 so this seems to be a longstanding bug. Pinging @colorful-tones @ndiego @kevin940726 for confidence check as I may have missed something critical. Also it looks like the PR needs a rebase against |
|
@getdave As of WordPress 6.3, the Post Editor is iframed if all registered blocks have a Block API version 3 or higher and no traditional metaboxes are registered. With WordPress 6.7 the editor is iframed also with metaboxes active: So we have a problem after the update. Some of our sites are no longer editable in wp-admin. There is no filter or anything to disable it. We need to add a block with API V2 to get an editor that works with relative URLs again. It's not a new bug, but it will now be visible to more users after the update to WordPress 6.7. |
|
Given the release's timing and the fact that this issue is not unique to 6.7, I would be inclined to punt to 6.7.1.
That said, I agree that this will likely impact many more people now that the Editor is iframed, even when metaboxes are active. To reduce the impact on users and developers, I recommend we try and include this in 6.7. |
|
I'm afraid I didn't understand the impact of the original Issue until a few moments ago. Given the potential scope I'd also be inclined to include. I see activity from @ellatrix and @youknowriad. How confident are you in this fix landing 6.7? |
|
It would have been good if the impact was included in the PR or issue description. Fwiw, I think this is safe. It will only affect relative URLs in the block canvas of links and resources. WordPress doesn't even normally use relative links. |
|
The failure is legit. This breaks the footnote back links, which are relative has urls. Since the So either we fix all links, or we only fix relative hash links. In both cases we'd have to intercept clicks on anchors tags in the iframe. |
|
@getdave At this part of the release cycle, only bug fixes for regressions introduced in-cycle can be included so this will need to wait until WP 6.8. |
|
With 6.6 it works. After the 6.7 update, all images in the backend are broken. I think that it should at least be included in 6.7.1. But perhaps we should wait and see whether we are a special case or whether it is a problem for other people. |
|
@peterwilsoncc Thanks for jumping in here. There's been some disagreement and discussion above about whether or not to try and include this, but yes this was an issue outside of the Beta / RC so on balance I think I stick by my original analysis - do not include in 6.7. I don't see any reason why we can't move to 6.7.1 though, unless there are some rules around what can be included in those point releases that I'm not aware of. |
karthick-murugan
pushed a commit
to karthick-murugan/gutenberg
that referenced
this pull request
Nov 13, 2024
Unlinked contributors: tambourine-man, Pat-Relentless, nicolasleroy. Co-authored-by: Soean <[email protected]> Co-authored-by: youknowriad <[email protected]> Co-authored-by: ellatrix <[email protected]> Co-authored-by: getdave <[email protected]> Co-authored-by: cweiske <[email protected]> Co-authored-by: peterwilsoncc <[email protected]>
Soean
added
the
Backport to WP Minor Release
|
I noticed a minor error with the fix - #68024. |
Soean
removed
the
Backport to WP Minor Release
|
Punting this to 6.8. As it created another minor error and is starting to diverge quite a lot. |
6 tasks
Mamaduka
added
the
Backport to WP Minor Release
|
@cbravobernal, added a minor release backport label back here. See: #68689. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Adds a
<base>element to the iframed editor to work with relative URLs.Fixes #54262
Rebase of #56533
Why?
Currently the editor in the iframe does not work with relative URLs.
Relative URLs work in the editor without iframe, where the media are loaded relative to the admin URL. The element restores the old behavior. In further iterations, we can make the editor even more flexible so that it can also work with other URLs.
Testing Instructions