Footnotes: use core’s meta revisioning if available #52988

adamsilverstein · 2023-07-26T15:59:00Z

What?

See:
https://core.trac.wordpress.org/ticket/20564 and
WordPress/wordpress-develop#4859

Why?

How?

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

adamsilverstein · 2023-07-26T20:12:01Z

@ellatrix Since we can't alter this file in core, we need to remove the revisioning here, right?

I wasn't sure if pattern was to use a WP version check or a capability check like I have here so you can keep the meta revisioning functionality for any older versions of WordPress. What is the correct approach? I can add some inline comments here indicating by the conditional logic is used.

Mamaduka · 2023-08-31T18:46:39Z

Hi, Adam

Do you mind releasing this branch on top of the current trunk?

# Conflicts: # packages/block-library/src/footnotes/index.php

adamsilverstein · 2023-09-08T19:56:36Z

@Mamaduka I rebased onto trunk and also added one line that stabilized the e2e tests. Can you give this another review please?

…eta changes

adamsilverstein · 2023-09-09T19:55:28Z

@Mamaduka & @ellatrix
I added a test in c18cae0 (#52988) that fails to illustrate the bug I mentioned on the other ticket: previewing changes to post meta also writes that meta to the published post. We should be able to preview meta changes without altering a published post.

adamsilverstein · 2023-09-09T20:44:59Z

with the changes in this PR copied over to src/wp-includes/blocks/footnotes.php, the issue with overwriting the published post meta goes away so I think there is something in the filters here causing the problem.

adamsilverstein · 2023-09-09T21:00:22Z

I added a fix for the live published post overwriting issue in 7d944d2 (#52988)

We can't use update_post_meta() to update the autosave's meta, because it doesn't allow revisions; it switches the id to the post parent (which is the live post) and saves the data there :(

You may want to consider back porting this small fix to previous releases (if you do that sort of thing).

adamsilverstein · 2023-09-11T04:39:02Z

@Mamaduka & @ellatrix I added a test in c18cae0 (#52988) that fails to illustrate the bug I mentioned on the other ticket: previewing changes to post meta also writes that meta to the published post. We should be able to preview meta changes without altering a published post.

I added this fix in this standalone PR: #54339 in case that is helpful

adamsilverstein · 2023-09-12T21:35:19Z

@Mamaduka I made some small additions to the core code to ensure the autosave changes work correctly. I found some of my testing was invalid because the plugin code was still active in core's footnotes.php file. I think the changes here need to be manually copied over there for testing.

Can you give this another test, also try commenting back in the last test which I commented out here so this can be merged until the core patch goes in.

Mamaduka · 2023-09-13T09:24:23Z

I left the update on the core PR - WordPress/wordpress-develop#4859 (comment).

adamsilverstein · 2023-09-26T15:47:44Z

@Mamaduka / @ellatrix - the core patch has landed in beta1 which should unlock this moving forward - https://core.trac.wordpress.org/changeset/56714

Mamaduka · 2023-09-26T15:50:12Z

Fantastic work, @adamsilverstein!

Do you mind rebasing this branch? Now that the code has shipped in the core let's see if anything needs adjustments.

adamsilverstein · 2023-09-26T16:06:16Z

@Mamaduka do the tests here run against trunk? If so, I should be able to uncomment the section of the 'can be previewed when published' that checks that a published post remains unchanged when previewing meta changes

Mamaduka · 2023-09-26T16:08:09Z

@adamsilverstein, yes, tests run against the WP trunk.

… changes

…viewing changes to meta.

This preveents the code from being included in core. This code can be removed once GB no longer supports core < 6.4.

adamsilverstein · 2023-09-28T17:14:38Z

@Mamaduka turns out the issue was slashing!

Rerunning tests without the slash test to confirm - temporarily in 7fee4f5

This will fix the issue: WordPress/wordpress-develop#5344

Also, I realized we should move all of the revision related functionality to the shim so we don't bring that over to core. I did that in 9d2f120 (#52988)

adamsilverstein · 2023-09-28T21:42:44Z

@Mamaduka E2E tests now pass (without the slash test). Once I merge the fix to core in WordPress/wordpress-develop#5344 we can add the slash test back. If you have a chance can you please review that PR? I added a test to validate the slashing bug as well as manual reproduction instructions.

~~I see some unit tests failures but it looks like flakey tests so I'll try rerunning.~~

All tests are now green :)

Mamaduka

Thank you, @adamsilverstein! 🦸

I assume commented-out tests can be enabled once the core PR is committed.

The PHP shims might need re-arrangement so Footnote revisions could work with WP < 6.4. I can do that after this is cherry-picked in beta two 👍

P.S. We can ignore the optional CI check; I am unsure why it's failing.

adamsilverstein · 2023-09-29T13:34:36Z

I assume commented-out tests can be enabled once the core PR is committed.

Yes, exactly.

adamsilverstein · 2023-09-29T15:08:51Z

I assume commented-out tests can be enabled once the core PR is committed.

Committed in https://core.trac.wordpress.org/changeset/56745 so all the tests should pass now, I will uncomment the slash tests.

adamsilverstein · 2023-09-29T15:13:43Z

The PHP shims might need re-arrangement so Footnote revisions could work with WP < 6.4. I can do that after this is cherry-picked in beta two 👍

Feel free to rework, I wasn't really sure what the best approach was and I didn't test in WP < 6.4. I do like the idea of a shim file that isn't ported over to core. I'd like to add the last bit of functionality that displays the meta on the revisions screen in core as well, so it won't need to be hooked in by the plugin. I'll work on that soon.

adamsilverstein · 2023-09-29T16:05:19Z

@Mamaduka - the slash test is restored and all E2E tests are green, this should be ready to go - shall I go ahead and click "Squash and Merge"?

Mamaduka · 2023-09-29T16:24:29Z

@adamsilverstein, sounds good 👍

# Conflicts: # packages/block-library/src/footnotes/index.php

mikachan · 2023-10-02T09:40:19Z

I just cherry-picked this PR to the commits-for-6.4-beta2 branch to get it included in the next release: 507d0e9

* Fix the install of system fonts from a font collection (#54713) * Fix set upload dir test (#54762) * Site Editor: Prevent unintended actions on the classic theme (#54422) * Add action and selector to track access to Patterns page * Add a URL parameter to check if the Patterns page was accessed directly * Revert lib changes * Fix critical error in the Post Editor * Explicitly add `! isBlockBasedTheme` with `isTemplatePartsMode` * Fix critical error in the Post Editor * Patterns: Fix back navigation after pattern creation (#54852) * Patterns: Fix category control width in site editor (#54853) * Patterns: Allow non-user patterns under Standard filter (#54756) * Performance Tests: Separate page setup from test (#53808) # Conflicts: # test/performance/specs/post-editor.spec.js * Performance Tests: Support legacy selector for expanded elements (#54690) * Paragraph: Make 'aria-label' consistent with other blocks (#54687) * Paragraph: Make 'aria-label' consistent with other blocks * Update tests * Try using BC labels in performance tests * Move lightbox render function to filter (#54670) * syntax refactor repace strpos with str_contains (#54832) * Font Library: avoid deprected error in test (#54802) * fix deprecated call * removing unwanted line * Fix the ShortcutProvider usage (#54851) Co-authored-by: Marin Atanasov <8436925+tyxla@users.noreply.github.com> * Image: Ensure `false` values are preserved in memory when defined in `theme.json` (#54639) * Modify conditional when parsing config * Only drop the value if it's undefined. --------- Co-authored-by: Michal Czaplinski <mmczaplinski@gmail.com> * Use "Not synced" in place of "Standard" nomenclature for patterns (#54839) * Standard -> Not synced * Fix broken test --------- Co-authored-by: Glen Davies <glen.davies@automattic.com> * Format Library: Try to fix highlight popover jumping (#54736) * Move mime-type collection generation to a function that can be tested… (#54844) * Move mime-type collection generation to a function that can be tested. Refactored to use that function. * linting changes * Add unit tests to mime type getter * Fixed linting errors * test the entire output array and replace assertTrue by assertEquals * fixing docs --------- Co-authored-by: Matias Benedetto <matias.benedetto@gmail.com> * Ensure lightbox toggle is shown if block-level setting exists (#54878) * Block Editor: Update strings in blocks 'RenameModal' component (#54887) * Footnotes: Add aria-label to return links (#54843) * Add aria-label to footnotes front-end links. * Add visual output. Fix PHPCS errors. * Remove visual changes, fix in follow-up. * Font Library: Changed the OTF mime type expected value to be what PHP returns (#54886) * Changed the OTF mime type expected value to be what PHP returns * add unit test for otf file installation --------- Co-authored-by: madhusudhand <madhusudhan.dollu@gmail.com> * Image: Fix layout shift when lightbox is opened and closed (#53026) * Replace overflow: hidden with JavaScript callback to prevent scrolling * Disable scroll callback on mobile; add comments; fix scrim styles The page jumps around when trying to override the scroll behavior on mobile, so I disabled it altogether. I've also added comments to clarify this and other decisions made around the scroll handling. While testing, I realized that the scrim was completely opaque during the zoom animation, which does not match the design, so I added a new animation specifically for the scrim to fix that. * Add handling for horizontally oriented pages * Move close button so that it's further from scrollbar on desktop * Fix call to handleScroll() and add touch callback to new render method * Improve lightbox experience on mobile To ensure pinch to zoom works as expected when the lightbox is open on mobile, I added logic to disable the scroll override when touch is detected. Without this, the scroll override kicks in whenever one tries to pinch to zoom, and it breaks the experience. I also revised the styles for the scrim to make it opaque, as having content visible outside of the lightbox is distracting when pinching to zoom on a mobile device, and I think having a consistent lightbox across devices will make for the best user experience. * Fix spacing * Add touch directives to block supports * Fix spacing in block supports * Rename attribute for clarity * Update comment * Update comments * Fix spacing --------- Co-authored-by: Ricardo Artemio Morales <ric.morales22@gmail.com> * Font Library: move font uploads to a new tab (#54655) * move font uploads to a new tab in the modal * fix invalid success message and revert the dropzone to modal * add success notice for font uploads * make supported file formats message dynamic based on allowed extensions * update hint text and clear successful message with a fresh upload * Block custom CSS: Fix incorrect CSS when multiple root selectors (#53602) * Block custom CSS: Fix incorrect CSS when multiple root selectors * Fix PHP lint error * Use `scope_selector` and `append_to_selector` method and update unit test * Use `scopeSelector` and `appendToSelector` function and update JS unit test * Update packages/block-editor/src/components/global-styles/test/use-global-styles-output.js Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> * Update packages/block-editor/src/components/global-styles/test/use-global-styles-output.js Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> * Update packages/block-editor/src/components/global-styles/test/use-global-styles-output.js Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> * Update packages/block-editor/src/components/global-styles/test/use-global-styles-output.js Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> * Update packages/block-editor/src/components/global-styles/utils.js Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> * re-trigger CI --------- Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> * Add new e2e test for creating a pattern (#54855) * Use list role instead of listbox for patterns (#54884) * Popover: Fix the styles for components that use emotion within popovers (#54912) # Conflicts: # packages/components/CHANGELOG.md * Footnotes: use core’s meta revisioning if available (#52988) # Conflicts: # packages/block-library/src/footnotes/index.php * Remove base url from link control search results (#54553) * Expose baseURL setting on Post and Site Editors via block settings * Strip baseURL from rendered search results * Only fetch baseURL once in top level component * Simplify implementation to utilise URL parse functions * Improve comment wording to avoid referencing undefined var * Remove superfluous conditional * Decode URL prior to operations * Refactor for readability * Fix where url is not defined * Revert change to filter util * Ensure that filterURLForDisplay always receives a string as an arg * Make e2e test locator less strict * Prefer pipe * Force remove trailing slash * [Site Editor]: Update copy of using the default template in a page (#54728) * Remove overflow: hidden from the entity title h1 in the site editor sidebar (#54769) * Site Editor: Avoid same key warnings in template parts area listings (#54863) * TemplateAreas use template part clientId as key * HomeTemplateDetails use template part clientId as key * Cleanup useSelect hook * Fix ToolSelector popover variant (#54840) * Font Library: refactor endpoint permissions (#54829) * break the checking of user permission and file write permissions * return error 500 if the request to the install fonts endpoint needs write permissions and wordpress doens't have write permission on the server * do not ask file write permission on uninstall endpoint * Standardize the output of install and uninstall fonts endpoints Co-authored-by: Jason Crist <146530+pbking@users.noreply.github.com> Co-authored-by: Jeff Ong <5375500+jffng@users.noreply.github.com> * Handle standardized output from install and uninstall endpoints in the frontend Co-authored-by: Jason Crist <146530+pbking@users.noreply.github.com> Co-authored-by: Jeff Ong <5375500+jffng@users.noreply.github.com> * Update the install and unintall fonts endpoints unit tests for the new standardized output format Co-authored-by: Jason Crist <146530+pbking@users.noreply.github.com> Co-authored-by: Jeff Ong <5375500+jffng@users.noreply.github.com> * fix the refersh call for the library Co-authored-by: Jason Crist <146530+pbking@users.noreply.github.com> Co-authored-by: Jeff Ong <5375500+jffng@users.noreply.github.com> --------- Co-authored-by: Jason Crist <146530+pbking@users.noreply.github.com> Co-authored-by: Jeff Ong <5375500+jffng@users.noreply.github.com> * Don’t use TypeScript files in scripts package (#54856) * Fix Search Block not updating in Nav block (#54823) * Avoid setState in render * Attempt at test coverage * Improve tests and make them work * Remove word-wrap property (#54866) --------- Co-authored-by: Matias Benedetto <matias.benedetto@gmail.com> Co-authored-by: Aki Hamano <54422211+t-hamano@users.noreply.github.com> Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Co-authored-by: Bart Kalisz <bartlomiej.kalisz@gmail.com> Co-authored-by: George Mamadashvili <georgemamadashvili@gmail.com> Co-authored-by: Artemio Morales <artemio.morales@a8c.com> Co-authored-by: Riad Benguella <benguella@gmail.com> Co-authored-by: Marin Atanasov <8436925+tyxla@users.noreply.github.com> Co-authored-by: Michal Czaplinski <mmczaplinski@gmail.com> Co-authored-by: Rich Tabor <hi@richtabor.com> Co-authored-by: Glen Davies <glen.davies@automattic.com> Co-authored-by: Jason Crist <jcrist@pbking.com> Co-authored-by: Alex Stine <alex.stine@yourtechadvisors.com> Co-authored-by: madhusudhand <madhusudhan.dollu@gmail.com> Co-authored-by: Carlos Bravo <37012961+c4rl0sbr4v0@users.noreply.github.com> Co-authored-by: Ricardo Artemio Morales <ric.morales22@gmail.com> Co-authored-by: Kai Hao <kevin830726@gmail.com> Co-authored-by: Adam Silverstein <adamjs@google.com> Co-authored-by: Dave Smith <getdavemail@gmail.com> Co-authored-by: Nik Tsekouras <ntsekouras@outlook.com> Co-authored-by: Ramon <ramonjd@users.noreply.github.com> Co-authored-by: Jason Crist <146530+pbking@users.noreply.github.com> Co-authored-by: Jeff Ong <5375500+jffng@users.noreply.github.com> Co-authored-by: Pascal Birchler <pascalb@google.com>

ellatrix · 2023-10-03T08:36:46Z

Great work @adamsilverstein, thank you!

youknowriad · 2024-01-24T07:53:49Z

lib/compat/plugin/footnotes.php

@@ -0,0 +1,250 @@
+<?php


@ellatrix If I'm not wrong this file should be moved to lib/compat/wordpress-6.4 because it's just here to support versions priori to 6.4 right?

Yes, although maybe I don't think these make sense anymore after the revision support was added. Cc @adamsilverstein

Footnotes: use core’s meta revisioning if available

8a169de

adamsilverstein requested a review from ajitbohra as a code owner July 26, 2023 15:59

skorasaurus added the [Block] Footnotes Affects the Footnotes Block label Jul 27, 2023

Mamaduka added the [Type] Enhancement A suggestion for improvement. label Aug 31, 2023

Mamaduka mentioned this pull request Aug 31, 2023

Add revisioning of post meta, including ‘footnotes’ by default WordPress/wordpress-develop#4859

Closed

adamsilverstein added 2 commits September 8, 2023 12:29

Merge branch 'trunk' into redo-footnote-revisioning

f8c55c2

# Conflicts: # packages/block-library/src/footnotes/index.php

Ensure preview menu is closed after opening preview

59bc54f

adamsilverstein requested review from kevin940726 and Mamaduka as code owners September 8, 2023 19:51

adamsilverstein added 2 commits September 9, 2023 09:52

Improve docs and add some new conditionals

fd9d4a0

Add test to verify published post meta is unchanged when previewing m…

c18cae0

…eta changes

Correct overwriting of live published post meta when previewing

7d944d2

adamsilverstein mentioned this pull request Sep 11, 2023

Fix overwriting of published post meta when previewing footnote changes #54339

Merged

Comment out tests that require core changes to pass

fb6d61e

sc0ttkclark approved these changes Sep 22, 2023

View reviewed changes

Merge branch 'trunk' into redo-footnote-revisioning

c91b95b

Include test to verify published post uneffected when previewing meta…

d7a96e1

… changes

adamsilverstein added 2 commits September 28, 2023 10:46

Restore test to verify that the published post is unchanged after pre…

2a55084

…viewing changes to meta.

Move the meta revision compat code to the footnotes shim file

352bba5

This preveents the code from being included in core. This code can be removed once GB no longer supports core < 6.4.

adamsilverstein mentioned this pull request Sep 28, 2023

Revisions: slash meta values for previews WordPress/wordpress-develop#5344

Closed

adamsilverstein added 6 commits September 28, 2023 11:16

remove hooks from footnotes file

de6882c

Ensure functions are guarded against redeclaration.

95f6b42

phpcbf

a8e98f6

phpcbf2

90057fc

restore typing “3”

c614958

remove extra space

90d7472

Mamaduka approved these changes Sep 29, 2023

View reviewed changes

Mamaduka added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Sep 29, 2023

Restore slashed data test

6a4f28b

adamsilverstein merged commit 66b25c1 into WordPress:trunk Sep 29, 2023
47 of 48 checks passed

adamsilverstein deleted the redo-footnote-revisioning branch September 29, 2023 16:40

github-actions bot added this to the Gutenberg 16.8 milestone Sep 29, 2023

mikachan pushed a commit that referenced this pull request Oct 2, 2023

Footnotes: use core’s meta revisioning if available (#52988)

507d0e9

# Conflicts: # packages/block-library/src/footnotes/index.php

mikachan removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Oct 2, 2023

annezazu mentioned this pull request Oct 12, 2023

Footnotes: ensure compatibility with Custom Post Types #52812

Closed

getdave mentioned this pull request Jan 18, 2024

Plugin: Synchronize PHP changes for WordPress 6.5 release #57959

Closed

youknowriad reviewed Jan 24, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Footnotes: use core’s meta revisioning if available #52988

Footnotes: use core’s meta revisioning if available #52988

adamsilverstein commented Jul 26, 2023 •

edited

Loading

adamsilverstein commented Jul 26, 2023

Mamaduka commented Aug 31, 2023

adamsilverstein commented Sep 8, 2023

adamsilverstein commented Sep 9, 2023 •

edited

Loading

adamsilverstein commented Sep 9, 2023

adamsilverstein commented Sep 9, 2023 •

edited

Loading

adamsilverstein commented Sep 11, 2023

adamsilverstein commented Sep 12, 2023

Mamaduka commented Sep 13, 2023

adamsilverstein commented Sep 26, 2023

Mamaduka commented Sep 26, 2023

adamsilverstein commented Sep 26, 2023

Mamaduka commented Sep 26, 2023

adamsilverstein commented Sep 28, 2023 •

edited

Loading

adamsilverstein commented Sep 28, 2023 •

edited

Loading

Mamaduka left a comment

adamsilverstein commented Sep 29, 2023

adamsilverstein commented Sep 29, 2023

adamsilverstein commented Sep 29, 2023

adamsilverstein commented Sep 29, 2023

Mamaduka commented Sep 29, 2023

mikachan commented Oct 2, 2023

ellatrix commented Oct 3, 2023

youknowriad Jan 24, 2024

ellatrix Jan 25, 2024

Footnotes: use core’s meta revisioning if available #52988

Footnotes: use core’s meta revisioning if available #52988

Conversation

adamsilverstein commented Jul 26, 2023 • edited Loading

What?

Why?

How?

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

adamsilverstein commented Jul 26, 2023

Mamaduka commented Aug 31, 2023

adamsilverstein commented Sep 8, 2023

adamsilverstein commented Sep 9, 2023 • edited Loading

adamsilverstein commented Sep 9, 2023

adamsilverstein commented Sep 9, 2023 • edited Loading

adamsilverstein commented Sep 11, 2023

adamsilverstein commented Sep 12, 2023

Mamaduka commented Sep 13, 2023

adamsilverstein commented Sep 26, 2023

Mamaduka commented Sep 26, 2023

adamsilverstein commented Sep 26, 2023

Mamaduka commented Sep 26, 2023

adamsilverstein commented Sep 28, 2023 • edited Loading

adamsilverstein commented Sep 28, 2023 • edited Loading

Mamaduka left a comment

Choose a reason for hiding this comment

adamsilverstein commented Sep 29, 2023

adamsilverstein commented Sep 29, 2023

adamsilverstein commented Sep 29, 2023

adamsilverstein commented Sep 29, 2023

Mamaduka commented Sep 29, 2023

mikachan commented Oct 2, 2023

ellatrix commented Oct 3, 2023

youknowriad Jan 24, 2024

Choose a reason for hiding this comment

ellatrix Jan 25, 2024

Choose a reason for hiding this comment

adamsilverstein commented Jul 26, 2023 •

edited

Loading

adamsilverstein commented Sep 9, 2023 •

edited

Loading

adamsilverstein commented Sep 9, 2023 •

edited

Loading

adamsilverstein commented Sep 28, 2023 •

edited

Loading

adamsilverstein commented Sep 28, 2023 •

edited

Loading