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

Use named parameters instead of $ and % in localised strings #2685

Merged
merged 5 commits into from
Oct 13, 2022

Conversation

absidue
Copy link
Member

@absidue absidue commented Oct 7, 2022

Use named parameters instead of $ and % in localised strings

Pull Request Type

  • Bugfix

Related issue

The codeql checks were added in #2650 (see the files tab on that pull request for the warnings).

Description

This pull reqest aims to fix the two issues reported by codeql: "Incomplete string escaping or encoding" and "Incomplete URL substring sanitization".

I wasn't sure whether I should change all of the language files, as Vue-i18n will happily use the old strings in languages that haven't been updated as, the key didn't change for all of the strings (if the key changes, vue-i18n can't find the translated string and falls back to the en-US one, which doesn't always happen here). I can definitely change all of the strings in the left-to-right languages as part of this pull request if you want me to, I won't be able to do right-to-left languages as the text seems to change entirely when I replace the dollar ($) or percent (%) symbols, so I definitely want to leave those up to the translators on weblate.

Testing

Ideally check every place where the changed strings show up, only en-US and en_GB will work properly until the other languages are updated. Although that might be too much testing for you so you could just check a couple of strings.

Desktop

  • OS: Windows
  • OS Version: 10
  • FreeTube version: 0.17.1

@PrestonN PrestonN enabled auto-merge (squash) October 7, 2022 16:05
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Oct 7, 2022
@PikachuEXE
Copy link
Collaborator

Should we also update other locale files or in separate PR?
Do other not updated locales work?

@absidue
Copy link
Member Author

absidue commented Oct 8, 2022

I'll update the other language files now, otherwise you'll see random $ and % in FreeTube until those languages are updated.

@absidue absidue mentioned this pull request Oct 9, 2022
1 task
@github-actions github-actions bot added PR: merge conflicts / rebase needed and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Oct 11, 2022
@github-actions
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

@absidue absidue added the PR: waiting for review For PRs that are complete, tested, and ready for review label Oct 11, 2022
Copy link
Member

@ChunkyProgrammer ChunkyProgrammer left a comment

Choose a reason for hiding this comment

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

Lgtm!

@PikachuEXE
Copy link
Collaborator

Not sure which file(s) but I got new errors in console
image

@github-actions github-actions bot added PR: merge conflicts / rebase needed and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Oct 12, 2022
@github-actions
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@absidue
Copy link
Member Author

absidue commented Oct 12, 2022

@PikachuEXE You can click on that link and it will show you the exact line in the code that caused the error, in this case we only use yaml-loader in one place, so I know where the error is happening. I'm guessing that some of the language files have YAML syntax errors.

@PikachuEXE
Copy link
Collaborator

PikachuEXE commented Oct 12, 2022

image

Even more expanded
image

@ChunkyProgrammer
Copy link
Member

Locales with issues: it, ja, ko, nb_NO

@ChunkyProgrammer
Copy link
Member

@absidue if you use vscode, I recommend installing this yaml extension to find the problems in the files (looks like quotes are missing around some values) https://marketplace.visualstudio.com/items?itemName=redhat.vscode-yaml

@github-actions
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

Copy link
Collaborator

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

Tested locally briefly with zh-TW

@PrestonN PrestonN merged commit c0f98ee into FreeTubeApp:development Oct 13, 2022
@absidue absidue deleted the fix-codeql-warnings branch October 13, 2022 12:05
MarmadileManteater added a commit to MarmadileManteater/FreeTubeAndroid that referenced this pull request Oct 18, 2022
commit 9733163
Merge: 25ff177 b5c486b
Author: Emma <MarmadileManteater@proton.me>

Merge branch 'FreeTubeApp:development' into development

commit b5c486b
Author: Emma <MarmadileManteater@proton.me>

Fixing some leftover `showToast(obj)` (FreeTubeApp#2735)

commit 198c3fa
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Bump http-proxy-agent from 4.0.1 to 5.0.0 (FreeTubeApp#2733)

commit 25ff177
Merge: 980a387 4ef6369
Author: Emma <MarmadileManteater@proton.me>

Merge branch 'upstream_development' into development

commit 4ef6369
Author: Gediminas Murauskas <muziejusinfo@gmail.com>

Translated using Weblate (Lithuanian)

commit 4440044
Author: Massimo Pissarello <mapi68@gmail.com>

Translated using Weblate (Italian)

commit 8209e7e
Author: Oğuz Ersen <oguz@ersen.moe>

Translated using Weblate (Turkish)

commit 3326224
Author: Jeff Huang <s8321414@gmail.com>

Translated using Weblate (Chinese (Traditional))

commit 546a276
Author: Eric <hamburger1024@mailbox.org>

Translated using Weblate (Chinese (Simplified))

commit 1b68e56
Author: Rex_sa <rex.sa@pm.me>

Translated using Weblate (Arabic)

commit 77a18e8
Merge: 62c70fb eefcd36
Author: Hosted Weblate <hosted@weblate.org>

Merge branch 'origin/development' into Weblate.

commit eefcd36
Author: Gediminas Murauskas <muziejusinfo@gmail.com>

Translated using Weblate (Lithuanian)

commit 94fb11f
Author: Grzegorz Wójcicki <terkaz@gmx.com>

Translated using Weblate (Polish)

commit 37b3b6d
Author: J. Lavoie <j.lavoie@net-c.ca>

Translated using Weblate (French)

commit 097c06a
Author: J. Lavoie <j.lavoie@net-c.ca>

Translated using Weblate (German)

commit 62c70fb
Author: absidue <48293849+absidue@users.noreply.github.com>

Fix fetching the comments from invidious (FreeTubeApp#2721)

commit 077fde7
Author: absidue <48293849+absidue@users.noreply.github.com>

Fix CTRL+clicking on the channel name on the watch page (FreeTubeApp#2713)

commit ad01d2b
Author: absidue <48293849+absidue@users.noreply.github.com>

Make the safe as and open file dialogs blocking (FreeTubeApp#2712)

commit 3aa080d
Author: absidue <48293849+absidue@users.noreply.github.com>

Make showToast a helper (FreeTubeApp#2695)

commit f94d8a9
Author: Emma <MarmadileManteater@proton.me>

Filtering out invidious instances that don't support the API (FreeTubeApp#2714)

commit 21a31cf
Author: PikachuEXE <pikachuexe@gmail.com>

Upgrade electron from 20 > 21 (FreeTubeApp#2717)

commit c0f98ee
Author: absidue <48293849+absidue@users.noreply.github.com>

Use named parameters instead of $ and % in localised strings (FreeTubeApp#2685)

commit 81426ed
Author: absidue <48293849+absidue@users.noreply.github.com>

Don't lint while rebasing (FreeTubeApp#2711)

commit 1695605
Author: Aiz <66974576+Aiz0@users.noreply.github.com>

Remove deprecated @keypress + some accessability improvements (FreeTubeApp#2697)

commit b35d7ea
Author: Sveinn í Felli <sv1@fellsnet.is>

Translated using Weblate (Icelandic)

commit 6b4727c
Merge: bc8261d9 2a60129
Author: Hosted Weblate <hosted@weblate.org>

Merge branch 'origin/development' into Weblate.

commit 2a60129
Author: Fjuro <ifjuro@proton.me>

Translated using Weblate (Czech)

commit 5199114
Author: ovari <ovari123@zoho.com>

Translated using Weblate (Hungarian)

commit 11b3c19
Author: Yaron Shahrabani <sh.yaron@gmail.com>

Translated using Weblate (Hebrew)

commit bd210ab
Author: Егор Ермаков <eg.ermakov2016@yandex.ru>

Translated using Weblate (Russian)

commit bc8261d
Author: absidue <48293849+absidue@users.noreply.github.com>

Move colours from the store to the utils helper (FreeTubeApp#2710)

commit f12b9e1
Author: SC <lalocas@protonmail.com>

Translated using Weblate (Portuguese)

**Full Changelog**: 0.17.1-nightly-68...0.17.1-nightly-69
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants