Skip to content
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

Link Suggestions Missing #45549

Closed
ChrissiePollock opened this issue Sep 10, 2020 · 30 comments
Closed

Link Suggestions Missing #45549

ChrissiePollock opened this issue Sep 10, 2020 · 30 comments
Assignees
Labels
[Goal] Gutenberg Working towards full integration with Gutenberg [Pri] High [Type] Bug

Comments

@ChrissiePollock
Copy link

ChrissiePollock commented Sep 10, 2020

Steps to reproduce

  1. Starting from the block editor for some WordPress.com sites (e.g. P4TzNm-3wQ-p2 or p7PFsI-7zI-p2).
  2. Write some words in the paragraph block.
  3. Select some text by highlighting it then click the paperclip icon to add a link.
  4. Search for a word you know you have in another post/post title. (I used the word "four.")
  5. Note there are no suggested posts to link to.

What I expected

I expected to see suggestions.

What happened instead

The box where suggestions usually go remained empty.

Browser / OS version

I tried on

Firefox - 80.0.1 (64-bit) and
Chrome - Version 85.0.4183.83 (Official Build) (64-bit)

Screenshot / Video

Screenshot -

screenshot

Screencast -

https://d.pr/i/sOjYFy (I'm not embedding since it just blinked when I did).

My console showed

screenshot

We'd also love to know how you found the bug: #user-report

@sirreal sirreal added [Goal] Gutenberg Working towards full integration with Gutenberg [Type] Bug labels Sep 10, 2020
@sirreal
Copy link
Member

sirreal commented Sep 10, 2020

When typing in the link search input, for working sites I see 2 network requests like this:

https://public-api.wordpress.com/wp/v2/sites/[SITE_ID]/search?_envelope=1&search=[SEARCH_TERM]&per_page=20&type=post&_gutenberg_nonce=[REDACTED]&_locale=user
https://public-api.wordpress.com/wp/v2/sites/[SITE_ID]/search?_envelope=1&search=[SEARCH_TERM]&per_page=20&type=term&_gutenberg_nonce=[REDACTED]&_locale=user

For sites experiencing this issue, a third request is issued which errors and I suspect is the culprit:

https://public-api.wordpress.com/wp/v2/sites/[SITE_ID]/search?_envelope=1&search=[SEARCH_TERM]&per_page=20&type=post&_gutenberg_nonce=[REDACTED]&_locale=user
https://public-api.wordpress.com/wp/v2/sites/[SITE_ID]/search?_envelope=1&search=[SEARCH_TERM]&per_page=20&type=term&_gutenberg_nonce=[REDACTED]&_locale=user

// ^^ Those are in the normal behavior
// vv This is unique to bad behavior

https://public-api.wordpress.com/wp/v2/sites/[SITE_ID]/search?_envelope=1&search=[SEARCH_TERM]&per_page=20&type=post-format&_gutenberg_nonce=[REDACTED]&_locale=user

The response to this bad request is like:

{
  "body": {
    "code": "rest_invalid_param",
    "message": "Invalid parameter(s): type",
    "data": {
      "status": 400,
      "params": {
        "type": "type is not one of post, term."
      }
    }
  },
  "status": 400,
  "headers": {
    "Allow": "GET"
  }
}

@sirreal
Copy link
Member

sirreal commented Sep 10, 2020

I think we'd want to start searching around here for why the additional search is being triggered: https://github.com/WordPress/gutenberg/blob/7e8ab3aa2d1db03b30480d6f224e9df5e5eb601e/packages/block-editor/src/components/link-control/search-input.js

@kcswong
Copy link

kcswong commented Sep 10, 2020

Had this happen to a user trying to add links to images. I checked my test site and was able to replicate on their theme Rosalie, as well as another premium theme Floral, but Aquene (also premium) didn't have the same issue. The free themes I tested (Independent Publisher 2 and Maywood) didn't have this issue either.

@ChrissiePollock
Copy link
Author

ChrissiePollock commented Sep 10, 2020

It was just a regular site I was testing on. Nothing special. I used the Purpose theme. This could be a theme glitch. When I switched to Twenty Twenty it worked as expected. The person who reported it to me had Twenty Sixteen as their theme.

@sirreal
Copy link
Member

sirreal commented Sep 10, 2020

Very helpful! It happens as soon as I activate Twenty Sixteen theme on my test site.

@ivan-ottinger
Copy link
Contributor

Another report in #3302530-zd - in this case it was a Twenty Fourteen theme.

→ Suggested the user to switch to a newer theme or copy and paste full post links.

@ivan-ottinger
Copy link
Contributor

Do you think this is something that should be reported to each theme separately? 🤔

@sirreal
Copy link
Member

sirreal commented Sep 11, 2020

I've just tested a .org site with Twenty Sixteen and I see the same 3 requests, however the third post-format request does not fail and results are displayed in the link suggestions correctly.

@sophiegyo
Copy link

I think we're also seeing the issue in 18683174-hc. User did not want to switch themes during the chat however.
They had a retired theme, Sonsa.

@sanjeev00733
Copy link

Another issue in #3303984-zen
They have Nikau theme active.

@joweber123
Copy link

Another report here #3305962-zd

Theme: Lovecraft

I can reproduce this with this theme as well.

@automattic-ian
Copy link

New report for #3312216-zd

Theme: Hemmingway Rewritten

@automattic-ian
Copy link

automattic-ian commented Sep 13, 2020

An additional report in #3302530-zd

Theme: twenty fourteen

@alshakero
Copy link
Member

alshakero commented Sep 14, 2020

Alright, I think I have the full picture. There are three points of failure here, fixing any of them would fix the issue:

  1. Some sites search for the query as a post-format: It is unclear to me why this is happening only now. But I know that it depends on a variable called disablePostFormats being false. I couldn't find how the value of this store property is originally determined. Although historically it was theme-dependent, which would make sense. But on master, I can't find what determines its value. It's worth mentioning that it defaults to false, which is the value that causes the failure.

  2. The endpoint fails badly: It fails with 400. Fixing the endpoint to return an empty array when it finds no results would fix the issue.

  3. Usage of Promise.all: We use Promise.all to run the 3 searches in parallel (searches against post, term, and post-format). This means that we discard all the successful searches if any of the 3 fails. I think we should use Promise.allSettled instead to losing data from successful promises when one of the three promises fail. I created a PR for this here.

@nerdysandy
Copy link
Contributor

reported on #24125300-hc

@khristiansnyder
Copy link

Reported in #24153733-hc
Theme: Nikau
Suggesting they manually add the links in the meantime.

@simison
Copy link
Member

simison commented Sep 16, 2020

The endpoint fails badly: It fails with 400. Fixing the endpoint to return an empty array when it finds no results would fix the issue.

@alshakero @p-jackson Can it be filtered to temporarily return empty array instead, so that problem would be fixed on .com? The 3rd option will still need to wait for Gutenberg release and could take weeks. 3rd option does seem like the best fix tho.

@aisajib
Copy link

aisajib commented Sep 16, 2020

This was reported in #14139879-hc
The site uses the premium theme Arcane.
Sent a follow-up in #3318792-zd suggesting that they manually copy the link for the time being.

@alshakero
Copy link
Member

alshakero commented Sep 16, 2020

A new finding. The backend side still checks for theme support before accepting 'post-format' query, whereas gutenberg's frontend doesn't. Causing a disparity and sending of faulty queries

Backend check: https://github.com/WordPress/gutenberg/blob/master/lib/rest-api.php#L338
Frontend check (now gone): https://github.com/WordPress/gutenberg/blob/5a0767e7228965b8297a3b04da6d4600b677ff9d/lib/client-assets.php#L901. Unless it is not checking somewhere else that I failed to find.

To clarify, the line in the backend code adds type=post-format to the allowed schema in the REST API.

@susanjanec
Copy link

Another user report 3311082-zd and I was also able to reproduce this issue on my own site.

@sourourn
Copy link

Another user report: #23584900-hc
Follow-up Ticket: #3323114-zen

@gwensmithx
Copy link

gwensmithx commented Sep 17, 2020

Another user report: #24207004-hc
Followup: #3326102-zd

Using Floral Theme. I was able to replicate this using the Floral Theme on my site and switching to Twenty Twenty solved it.

@supernovia
Copy link
Contributor

I just saw this while testing Gutenberg Edge. Site uses Baskerville.

@p-jackson
Copy link
Member

p-jackson commented Sep 18, 2020

I'm working on a "fix" that returns an empty array for type=post-format to fix suggestions on wpcom before @alshakero's real fix is shipped in GB.

D49783-code

@retnonindya
Copy link

Another user report: 3320854-zd -- the site is using Baskerville 2 theme

@alshakero
Copy link
Member

alshakero commented Sep 18, 2020

I think I understand the issue better now. In this PR from 24 days ago the post-format searching feature is added.

In this commit, the Gutenberg search handler only registers the type=post-format handler when current_theme_supports( 'post-formats' ) is true. Which makes sense at a first glance, but placing some error_logs there showed that current_theme_supports( 'post-formats' ) will never be true because this function doesn't know which site is calling, thus it doesn't know which theme is activated, thus what is and isn't supported by the theme. This is at least true in wpcom (since API calls don't have the same domain as the blog itself).

Adding error_log ( get_current_theme() ), gave me "Theme Name: WordPress.com REST API placeholder.".
And the current blog ID is the public-api blog ID.

@simison
Copy link
Member

simison commented Sep 18, 2020

Typically in those cases we do:

switch_to_blog( $blog_id );

// Do the thing

restore_current_blog();

@alshakero
Copy link
Member

Yes I found this function but it didn't make a difference. I hardcoded the blog id like so:

switch_to_blog( 565465 ); // 565465 is arbitary

And the current_theme_supports call didn't pick up the theme still.

@p-jackson
Copy link
Member

p-jackson commented Sep 20, 2020

I've deployed D49783-code so the issue should be fixed for simple site users now. I've checked all the reports linked above and luckily they're all simple sites afaict.

@p-jackson
Copy link
Member

Closing issue as this should now be fixed for users on wpcom. Please feel free to re-open if that's not the case!

WordPress/gutenberg#25302 is technically tracking the underlying issue in the GB repo, which feels like a more appropriate place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Goal] Gutenberg Working towards full integration with Gutenberg [Pri] High [Type] Bug
Projects
None yet
Development

No branches or pull requests