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

Jetpack Search: Does not work when the site language is Arabic #21349

Closed
mrsjaved opened this issue Oct 7, 2021 · 7 comments · Fixed by #21367
Closed

Jetpack Search: Does not work when the site language is Arabic #21349

mrsjaved opened this issue Oct 7, 2021 · 7 comments · Fixed by #21367
Assignees
Labels
Customer Report Issues or PRs that were reported via Happiness. aka "Happiness Request", or "User Report" [Feature] Search For all things related to Search [Focus] i18n Internationalization / i18n, adaptation to different languages [Pri] High [Type] Bug When a feature is broken and / or not performing as intended

Comments

@mrsjaved
Copy link

mrsjaved commented Oct 7, 2021

Quick summary

Jetpack search does not work when the site language is set to Arabic.

Steps to reproduce

  1. Make sure JP search is eanbled from JP settings -> Performance -> Search
  2. Change site language to Arabic (no need to change it for the user interface) from site settings -> general.
  3. Add JP search widget in the sidebar and try to search.

What you expected to happen

It should display results in popup

What actually happened

Does nothing.

Context

No response

Operating System

No response

Browser

No response

Simple, Atomic or both?

Atomic

Theme-specific issue?

No response

Other notes

No response

Reproducibility

Consistent

Severity

Some (< 50%)

Available workarounds?

No response

Workaround details

No response

Ticket: 4357314-zd-woothemes

@Robertght
Copy link

Thank you for reporting this @mrsjaved !

I tested this on a Jetpack connected site and I think it's related to the Jetpack Search module as I wasn't able to get the pop-up on my end after changing the language. It does seem to default to the classic search flow instead as it redirects me to a search page and it also has no results, compared to the previous search(made when the site was set to English).

Can this be a more general issue with the RTL websites @jeherve ?

@jeherve
Copy link
Member

jeherve commented Oct 8, 2021

Can this be a more general issue with the RTL websites

Yes, it would seem like it. I'll transfer this issue to the Jetpack repo, where Jetpack Search is built.

@jeherve jeherve transferred this issue from Automattic/wp-calypso Oct 8, 2021
@matticbot matticbot added the Customer Report Issues or PRs that were reported via Happiness. aka "Happiness Request", or "User Report" label Oct 8, 2021
@jeherve jeherve added [Pri] High [Type] Bug When a feature is broken and / or not performing as intended Instant Search [Focus] i18n Internationalization / i18n, adaptation to different languages [Feature] Search For all things related to Search labels Oct 8, 2021
@jeherve
Copy link
Member

jeherve commented Oct 8, 2021

cc @anomiex who also worked on #21235.

@anomiex
Copy link
Contributor

anomiex commented Oct 11, 2021

Trying it locally, I see this in my browser console:

Uncaught (in promise) Error: Loading CSS chunk main-payload failed.
(https://REDACTED/wp-content/plugins/jetpack/_inc/build/instant-search/jp-search.chunk-main-payload.min.rtl.css?ver=f33ebf8feb95ae7fc64a)

Looks like the problem is that @automattic/webpack-rtl-plugin expects the CSS asset's name to end in .css, while since #20926 it's ending in something like .css?ver=f33ebf8feb95ae7fc64a. So it never produces the .rtl.css file, which makes Instant Search fail to load for RTL languages.

cc @kangzj and @jsnmoon who were involved with that PR.

@kangzj
Copy link
Contributor

kangzj commented Oct 11, 2021

thanks @anomiex I'm onto it.

@Robertght
Copy link

Robertght commented Oct 13, 2021

Another case in #4365458-zen

*User wants to get info when solved.

@jsnmoon
Copy link
Contributor

jsnmoon commented Oct 18, 2021

@mrsjaved & @Robertght: The bug fix should ship with Jetpack 10.2.1, which should be released within 24-48 hours at the time of writing.

anomiex added a commit that referenced this issue Oct 20, 2021
This is intended to (eventually) take care of several issues in the
monorepo:

* Calypso [doesn't use their own calyspo-build webpack config][1],
  which means it's not likely to be well-maintained.
  * It needs a hack to [work with monorepo packages][2].
  * And another to [get correct image paths][3].
  * And it [tries to redefine NODE_ENV, producing warnings][4].
  * And we have to [override a weird default everywhere][5].
* We have to [name our JS files with ".min.js"][6] to avoid their being
  broken by a wpcom minifier, but WordPress.org's translation
  infrastructure [ignores such files][7].
* The plugin used to fix the above [had to be forked][8], adds 772K to
  the plugin, and results in spurious changes in TC builds.
* The way translations are [being loaded for lazy-loaded bundles][9] is
  pretty hacky and doesn't lazy-load them.
* We can't use any cache busting for the lazy-loaded bundles, because
  embedding hashes in the filename breaks the link to the translation
  file and putting it in a query parameter [breaks building the RTL css][10].
* Calypso-build has a lot of peer dependencies we don't actually need.
  Not everything we have uses sass or postcss, and we have our own
  infrastructure for jest, react, and so on. Currently we just ignore
  the 76 warnings from that.

This is the first step towards fixing that: creating a private package
that can hold the webpack and babel config fragments that we can share
throughout the monorepo. Then we'll have a place to put the fixes for
some of the other issues too.

[1]: #21004 (comment)
[2]: Automattic/wp-calypso#53353
[3]: Automattic/wp-calypso#56111
[4]: Automattic/wp-calypso#56291
[5]: #21004
[6]: #20484
[7]: #21343
[8]: https://github.com/Automattic/jetpack/blob/7a5edd83/projects/plugins/jetpack/tools/webpack.helpers.js#L35
[9]: #20926
[10]: #21349
anomiex added a commit that referenced this issue Oct 20, 2021
This is intended to (eventually) take care of several issues in the
monorepo:

* Calypso [doesn't use their own calyspo-build webpack config][1],
  which means it's not likely to be well-maintained.
  * It needs a hack to [work with monorepo packages][2].
  * And another to [get correct image paths][3].
  * And it [tries to redefine NODE_ENV, producing warnings][4].
  * And we have to [override a weird default everywhere][5].
* We have to [name our JS files with ".min.js"][6] to avoid their being
  broken by a wpcom minifier, but WordPress.org's translation
  infrastructure [ignores such files][7].
* The plugin used to fix the above [had to be forked][8], adds 772K to
  the plugin, and results in spurious changes in TC builds.
* The way translations are [being loaded for lazy-loaded bundles][9] is
  pretty hacky and doesn't lazy-load them.
* We can't use any cache busting for the lazy-loaded bundles, because
  embedding hashes in the filename breaks the link to the translation
  file and putting it in a query parameter [breaks building the RTL css][10].
* Calypso-build has a lot of peer dependencies we don't actually need.
  Not everything we have uses sass or postcss, and we have our own
  infrastructure for jest, react, and so on. Currently we just ignore
  the 76 warnings from that.

This is the first step towards fixing that: creating a private package
that can hold the webpack and babel config fragments that we can share
throughout the monorepo. Then we'll have a place to put the fixes for
some of the other issues too.

[1]: #21004 (comment)
[2]: Automattic/wp-calypso#53353
[3]: Automattic/wp-calypso#56111
[4]: Automattic/wp-calypso#56291
[5]: #21004
[6]: #20484
[7]: #21343
[8]: https://github.com/Automattic/jetpack/blob/7a5edd83/projects/plugins/jetpack/tools/webpack.helpers.js#L35
[9]: #20926
[10]: #21349
anomiex added a commit that referenced this issue Oct 25, 2021
This is intended to (eventually) take care of several issues in the
monorepo:

* Calypso [doesn't use their own calyspo-build webpack config][1],
  which means it's not likely to be well-maintained.
  * It needs a hack to [work with monorepo packages][2].
  * And another to [get correct image paths][3].
  * And it [tries to redefine NODE_ENV, producing warnings][4].
  * And we have to [override a weird default everywhere][5].
* We have to [name our JS files with ".min.js"][6] to avoid their being
  broken by a wpcom minifier, but WordPress.org's translation
  infrastructure [ignores such files][7].
* The plugin used to fix the above [had to be forked][8], adds 772K to
  the plugin, and results in spurious changes in TC builds.
* The way translations are [being loaded for lazy-loaded bundles][9] is
  pretty hacky and doesn't lazy-load them.
* We can't use any cache busting for the lazy-loaded bundles, because
  embedding hashes in the filename breaks the link to the translation
  file and putting it in a query parameter [breaks building the RTL css][10].
* Webpack's minification is [losing translator comments][11].
* Calypso-build has a lot of peer dependencies we don't actually need.
  Not everything we have uses sass or postcss, and we have our own
  infrastructure for jest, react, and so on. Currently we just ignore
  the 76 warnings from that.

This is the first step towards fixing that: creating a private package
that can hold the webpack and babel config fragments that we can share
throughout the monorepo. Then we'll have a place to put the fixes for
some of the other issues too.

[1]: #21004 (comment)
[2]: Automattic/wp-calypso#53353
[3]: Automattic/wp-calypso#56111
[4]: Automattic/wp-calypso#56291
[5]: #21004
[6]: #20484
[7]: #21343
[8]: https://github.com/Automattic/jetpack/blob/7a5edd83/projects/plugins/jetpack/tools/webpack.helpers.js#L35
[9]: #20926
[10]: #21349
[11]: #16549
anomiex added a commit that referenced this issue Oct 28, 2021
This is intended to (eventually) take care of several issues in the
monorepo:

* Calypso [doesn't use their own calyspo-build webpack config][1],
  which means it's not likely to be well-maintained.
  * It needs a hack to [work with monorepo packages][2].
  * And another to [get correct image paths][3].
  * And it [tries to redefine NODE_ENV, producing warnings][4].
  * And we have to [override a weird default everywhere][5].
* We have to [name our JS files with ".min.js"][6] to avoid their being
  broken by a wpcom minifier, but WordPress.org's translation
  infrastructure [ignores such files][7].
* The plugin used to fix the above [had to be forked][8], adds 772K to
  the plugin, and results in spurious changes in TC builds.
* The way translations are [being loaded for lazy-loaded bundles][9] is
  pretty hacky and doesn't lazy-load them.
* We can't use any cache busting for the lazy-loaded bundles, because
  embedding hashes in the filename breaks the link to the translation
  file and putting it in a query parameter [breaks building the RTL css][10].
* Webpack's minification is [losing translator comments][11].
* Calypso-build has a lot of peer dependencies we don't actually need.
  Not everything we have uses sass or postcss, and we have our own
  infrastructure for jest, react, and so on. Currently we just ignore
  the 76 warnings from that.

This is the first step towards fixing that: creating a private package
that can hold the webpack and babel config fragments that we can share
throughout the monorepo. Then we'll have a place to put the fixes for
some of the other issues too.

[1]: #21004 (comment)
[2]: Automattic/wp-calypso#53353
[3]: Automattic/wp-calypso#56111
[4]: Automattic/wp-calypso#56291
[5]: #21004
[6]: #20484
[7]: #21343
[8]: https://github.com/Automattic/jetpack/blob/7a5edd83/projects/plugins/jetpack/tools/webpack.helpers.js#L35
[9]: #20926
[10]: #21349
[11]: #16549
anomiex added a commit to Automattic/wp-calypso that referenced this issue Oct 29, 2021
For example, for cache busting of on-demand modules while still having
stable filenames you might have your CSS chunkFilename be like
`[name].css?ver=[contenthash]`.

See Automattic/jetpack#21349 for example.
kraftbj pushed a commit that referenced this issue Nov 1, 2021
This is intended to (eventually) take care of several issues in the
monorepo:

* Calypso [doesn't use their own calyspo-build webpack config][1],
  which means it's not likely to be well-maintained.
  * It needs a hack to [work with monorepo packages][2].
  * And another to [get correct image paths][3].
  * And it [tries to redefine NODE_ENV, producing warnings][4].
  * And we have to [override a weird default everywhere][5].
* We have to [name our JS files with ".min.js"][6] to avoid their being
  broken by a wpcom minifier, but WordPress.org's translation
  infrastructure [ignores such files][7].
* The plugin used to fix the above [had to be forked][8], adds 772K to
  the plugin, and results in spurious changes in TC builds.
* The way translations are [being loaded for lazy-loaded bundles][9] is
  pretty hacky and doesn't lazy-load them.
* We can't use any cache busting for the lazy-loaded bundles, because
  embedding hashes in the filename breaks the link to the translation
  file and putting it in a query parameter [breaks building the RTL css][10].
* Webpack's minification is [losing translator comments][11].
* Calypso-build has a lot of peer dependencies we don't actually need.
  Not everything we have uses sass or postcss, and we have our own
  infrastructure for jest, react, and so on. Currently we just ignore
  the 76 warnings from that.

This is the first step towards fixing that: creating a private package
that can hold the webpack and babel config fragments that we can share
throughout the monorepo. Then we'll have a place to put the fixes for
some of the other issues too.

[1]: #21004 (comment)
[2]: Automattic/wp-calypso#53353
[3]: Automattic/wp-calypso#56111
[4]: Automattic/wp-calypso#56291
[5]: #21004
[6]: #20484
[7]: #21343
[8]: https://github.com/Automattic/jetpack/blob/7a5edd83/projects/plugins/jetpack/tools/webpack.helpers.js#L35
[9]: #20926
[10]: #21349
[11]: #16549
matticbot pushed a commit to Automattic/jetpack-storybook that referenced this issue Nov 1, 2021
This is intended to (eventually) take care of several issues in the
monorepo:

* Calypso [doesn't use their own calyspo-build webpack config][1],
  which means it's not likely to be well-maintained.
  * It needs a hack to [work with monorepo packages][2].
  * And another to [get correct image paths][3].
  * And it [tries to redefine NODE_ENV, producing warnings][4].
  * And we have to [override a weird default everywhere][5].
* We have to [name our JS files with ".min.js"][6] to avoid their being
  broken by a wpcom minifier, but WordPress.org's translation
  infrastructure [ignores such files][7].
* The plugin used to fix the above [had to be forked][8], adds 772K to
  the plugin, and results in spurious changes in TC builds.
* The way translations are [being loaded for lazy-loaded bundles][9] is
  pretty hacky and doesn't lazy-load them.
* We can't use any cache busting for the lazy-loaded bundles, because
  embedding hashes in the filename breaks the link to the translation
  file and putting it in a query parameter [breaks building the RTL css][10].
* Webpack's minification is [losing translator comments][11].
* Calypso-build has a lot of peer dependencies we don't actually need.
  Not everything we have uses sass or postcss, and we have our own
  infrastructure for jest, react, and so on. Currently we just ignore
  the 76 warnings from that.

This is the first step towards fixing that: creating a private package
that can hold the webpack and babel config fragments that we can share
throughout the monorepo. Then we'll have a place to put the fixes for
some of the other issues too.

[1]: Automattic/jetpack#21004 (comment)
[2]: Automattic/wp-calypso#53353
[3]: Automattic/wp-calypso#56111
[4]: Automattic/wp-calypso#56291
[5]: Automattic/jetpack#21004
[6]: Automattic/jetpack#20484
[7]: Automattic/jetpack#21343
[8]: https://github.com/Automattic/jetpack/blob/7a5edd83/projects/plugins/jetpack/tools/webpack.helpers.js#L35
[9]: Automattic/jetpack#20926
[10]: Automattic/jetpack#21349
[11]: Automattic/jetpack#16549

Committed via a GitHub action: https://github.com/Automattic/jetpack/actions/runs/1409697994
matticbot pushed a commit to Automattic/jetpack-connection-ui that referenced this issue Nov 1, 2021
This is intended to (eventually) take care of several issues in the
monorepo:

* Calypso [doesn't use their own calyspo-build webpack config][1],
  which means it's not likely to be well-maintained.
  * It needs a hack to [work with monorepo packages][2].
  * And another to [get correct image paths][3].
  * And it [tries to redefine NODE_ENV, producing warnings][4].
  * And we have to [override a weird default everywhere][5].
* We have to [name our JS files with ".min.js"][6] to avoid their being
  broken by a wpcom minifier, but WordPress.org's translation
  infrastructure [ignores such files][7].
* The plugin used to fix the above [had to be forked][8], adds 772K to
  the plugin, and results in spurious changes in TC builds.
* The way translations are [being loaded for lazy-loaded bundles][9] is
  pretty hacky and doesn't lazy-load them.
* We can't use any cache busting for the lazy-loaded bundles, because
  embedding hashes in the filename breaks the link to the translation
  file and putting it in a query parameter [breaks building the RTL css][10].
* Webpack's minification is [losing translator comments][11].
* Calypso-build has a lot of peer dependencies we don't actually need.
  Not everything we have uses sass or postcss, and we have our own
  infrastructure for jest, react, and so on. Currently we just ignore
  the 76 warnings from that.

This is the first step towards fixing that: creating a private package
that can hold the webpack and babel config fragments that we can share
throughout the monorepo. Then we'll have a place to put the fixes for
some of the other issues too.

[1]: Automattic/jetpack#21004 (comment)
[2]: Automattic/wp-calypso#53353
[3]: Automattic/wp-calypso#56111
[4]: Automattic/wp-calypso#56291
[5]: Automattic/jetpack#21004
[6]: Automattic/jetpack#20484
[7]: Automattic/jetpack#21343
[8]: https://github.com/Automattic/jetpack/blob/7a5edd83/projects/plugins/jetpack/tools/webpack.helpers.js#L35
[9]: Automattic/jetpack#20926
[10]: Automattic/jetpack#21349
[11]: Automattic/jetpack#16549

Committed via a GitHub action: https://github.com/Automattic/jetpack/actions/runs/1409697994
matticbot pushed a commit to Automattic/jetpack-identity-crisis that referenced this issue Nov 1, 2021
This is intended to (eventually) take care of several issues in the
monorepo:

* Calypso [doesn't use their own calyspo-build webpack config][1],
  which means it's not likely to be well-maintained.
  * It needs a hack to [work with monorepo packages][2].
  * And another to [get correct image paths][3].
  * And it [tries to redefine NODE_ENV, producing warnings][4].
  * And we have to [override a weird default everywhere][5].
* We have to [name our JS files with ".min.js"][6] to avoid their being
  broken by a wpcom minifier, but WordPress.org's translation
  infrastructure [ignores such files][7].
* The plugin used to fix the above [had to be forked][8], adds 772K to
  the plugin, and results in spurious changes in TC builds.
* The way translations are [being loaded for lazy-loaded bundles][9] is
  pretty hacky and doesn't lazy-load them.
* We can't use any cache busting for the lazy-loaded bundles, because
  embedding hashes in the filename breaks the link to the translation
  file and putting it in a query parameter [breaks building the RTL css][10].
* Webpack's minification is [losing translator comments][11].
* Calypso-build has a lot of peer dependencies we don't actually need.
  Not everything we have uses sass or postcss, and we have our own
  infrastructure for jest, react, and so on. Currently we just ignore
  the 76 warnings from that.

This is the first step towards fixing that: creating a private package
that can hold the webpack and babel config fragments that we can share
throughout the monorepo. Then we'll have a place to put the fixes for
some of the other issues too.

[1]: Automattic/jetpack#21004 (comment)
[2]: Automattic/wp-calypso#53353
[3]: Automattic/wp-calypso#56111
[4]: Automattic/wp-calypso#56291
[5]: Automattic/jetpack#21004
[6]: Automattic/jetpack#20484
[7]: Automattic/jetpack#21343
[8]: https://github.com/Automattic/jetpack/blob/7a5edd83/projects/plugins/jetpack/tools/webpack.helpers.js#L35
[9]: Automattic/jetpack#20926
[10]: Automattic/jetpack#21349
[11]: Automattic/jetpack#16549

Committed via a GitHub action: https://github.com/Automattic/jetpack/actions/runs/1409697994
matticbot pushed a commit to Automattic/jetpack-backup-plugin that referenced this issue Nov 1, 2021
This is intended to (eventually) take care of several issues in the
monorepo:

* Calypso [doesn't use their own calyspo-build webpack config][1],
  which means it's not likely to be well-maintained.
  * It needs a hack to [work with monorepo packages][2].
  * And another to [get correct image paths][3].
  * And it [tries to redefine NODE_ENV, producing warnings][4].
  * And we have to [override a weird default everywhere][5].
* We have to [name our JS files with ".min.js"][6] to avoid their being
  broken by a wpcom minifier, but WordPress.org's translation
  infrastructure [ignores such files][7].
* The plugin used to fix the above [had to be forked][8], adds 772K to
  the plugin, and results in spurious changes in TC builds.
* The way translations are [being loaded for lazy-loaded bundles][9] is
  pretty hacky and doesn't lazy-load them.
* We can't use any cache busting for the lazy-loaded bundles, because
  embedding hashes in the filename breaks the link to the translation
  file and putting it in a query parameter [breaks building the RTL css][10].
* Webpack's minification is [losing translator comments][11].
* Calypso-build has a lot of peer dependencies we don't actually need.
  Not everything we have uses sass or postcss, and we have our own
  infrastructure for jest, react, and so on. Currently we just ignore
  the 76 warnings from that.

This is the first step towards fixing that: creating a private package
that can hold the webpack and babel config fragments that we can share
throughout the monorepo. Then we'll have a place to put the fixes for
some of the other issues too.

[1]: Automattic/jetpack#21004 (comment)
[2]: Automattic/wp-calypso#53353
[3]: Automattic/wp-calypso#56111
[4]: Automattic/wp-calypso#56291
[5]: Automattic/jetpack#21004
[6]: Automattic/jetpack#20484
[7]: Automattic/jetpack#21343
[8]: https://github.com/Automattic/jetpack/blob/7a5edd83/projects/plugins/jetpack/tools/webpack.helpers.js#L35
[9]: Automattic/jetpack#20926
[10]: Automattic/jetpack#21349
[11]: Automattic/jetpack#16549

Committed via a GitHub action: https://github.com/Automattic/jetpack/actions/runs/1409697994
matticbot pushed a commit to Automattic/jetpack-production that referenced this issue Nov 1, 2021
This is intended to (eventually) take care of several issues in the
monorepo:

* Calypso [doesn't use their own calyspo-build webpack config][1],
  which means it's not likely to be well-maintained.
  * It needs a hack to [work with monorepo packages][2].
  * And another to [get correct image paths][3].
  * And it [tries to redefine NODE_ENV, producing warnings][4].
  * And we have to [override a weird default everywhere][5].
* We have to [name our JS files with ".min.js"][6] to avoid their being
  broken by a wpcom minifier, but WordPress.org's translation
  infrastructure [ignores such files][7].
* The plugin used to fix the above [had to be forked][8], adds 772K to
  the plugin, and results in spurious changes in TC builds.
* The way translations are [being loaded for lazy-loaded bundles][9] is
  pretty hacky and doesn't lazy-load them.
* We can't use any cache busting for the lazy-loaded bundles, because
  embedding hashes in the filename breaks the link to the translation
  file and putting it in a query parameter [breaks building the RTL css][10].
* Webpack's minification is [losing translator comments][11].
* Calypso-build has a lot of peer dependencies we don't actually need.
  Not everything we have uses sass or postcss, and we have our own
  infrastructure for jest, react, and so on. Currently we just ignore
  the 76 warnings from that.

This is the first step towards fixing that: creating a private package
that can hold the webpack and babel config fragments that we can share
throughout the monorepo. Then we'll have a place to put the fixes for
some of the other issues too.

[1]: Automattic/jetpack#21004 (comment)
[2]: Automattic/wp-calypso#53353
[3]: Automattic/wp-calypso#56111
[4]: Automattic/wp-calypso#56291
[5]: Automattic/jetpack#21004
[6]: Automattic/jetpack#20484
[7]: Automattic/jetpack#21343
[8]: https://github.com/Automattic/jetpack/blob/7a5edd83/projects/plugins/jetpack/tools/webpack.helpers.js#L35
[9]: Automattic/jetpack#20926
[10]: Automattic/jetpack#21349
[11]: Automattic/jetpack#16549

Committed via a GitHub action: https://github.com/Automattic/jetpack/actions/runs/1409697994
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Customer Report Issues or PRs that were reported via Happiness. aka "Happiness Request", or "User Report" [Feature] Search For all things related to Search [Focus] i18n Internationalization / i18n, adaptation to different languages [Pri] High [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants