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

chore: revert uneffective code for encoding of page title #3768

Merged
merged 1 commit into from
Apr 24, 2023

Conversation

rob006
Copy link
Contributor

@rob006 rob006 commented Mar 18, 2023

This PR effectively reverts #3542 and #3684, as both these fixes does not work correctly seems to bring more harm than good. The real issue is described in #3685 and it should be fixed on trans() level.

Use case:

  1. Set forum title to Flarum & Flarum.
  2. Create new discussion with title & & & test.

Before this PR title displayed in browser tab would be: & & & test - Flarum & Flarum.
After this PR: & & & test - Flarum & Flarum.
Correct title should be: & & & test - Flarum & Flarum.

While this PR does not fix issue completely, it makes things better. Currently even on Discuss some discussions have incorrectly encoded title:

b6037cb7

Necessity

  • Has the problem that is being solved here been clearly explained?
  • If applicable, have various options for solving this problem been considered?
  • For core PRs, does this need to be in core, or could it be in an extension?
  • Are we willing to maintain this for years / potentially forever?

Confirmed

  • Frontend changes: tested on a local Flarum installation.
  • Backend changes: tests are green (run composer test).
  • Core developer confirmed locally this works as intended.
  • Tests have been added, or are not appropriate here.

Required changes:

  • Related documentation PR: (Remove if irrelevant)
  • Related core extension PRs: (Remove if irrelevant)

@rob006 rob006 requested a review from a team as a code owner March 18, 2023 22:38
@imorland imorland requested a review from SychO9 April 19, 2023 09:51
@SychO9 SychO9 changed the title Fix encoding of page title chore: revert uneffective code for encoding of page title Apr 24, 2023
Copy link
Member

@SychO9 SychO9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have sworn the last PR fixed this for good with the title, but considering the source of the issue is elsewhere, it would be better fixed there.

@imorland imorland added this to the 1.8 milestone Apr 24, 2023
@imorland imorland merged commit fea31a8 into flarum:main Apr 24, 2023
@rob006 rob006 deleted the page-title branch April 24, 2023 17:55
@github-actions github-actions bot mentioned this pull request May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants