For #8017: fix wifi string

[severinrudie]

Pull Request checklist

• Tests: This PR includes thorough tests or an explanation of why it does not
• Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not
• Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features. In addition, it includes a screenshot of a successful accessibility scan to ensure no new defects are added to the product.

After merge

• Milestone: Make sure issues finished by this pull request are added to the milestone of the version currently in development.

To download an APK when reviewing a PR:

1. click on Show All Checks,
2. click Details next to "Taskcluster (pull_request)" after it appears and then finishes with a green checkmark,
3. click on the "Fenix - assemble" task, then click "Run Artifacts".
4. the APK links should be on the left side of the screen, named for each CPU architecture

[severinrudie]

@betsymi would you mind verifying these strings before we merge? Please lmk if you would like any additional changes. These mocks provide some context of where they will be used in app (#8017).

[codecov-io]

Codecov Report

Merging #8953 into master will decrease coverage by 0.03%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #8953      +/-   ##
============================================
- Coverage     18.96%   18.92%   -0.04%     
  Complexity      500      500              
============================================
  Files           330      330              
  Lines         13027    13027              
  Branches       1725     1725              
============================================
- Hits           2470     2466       -4     
- Misses        10346    10351       +5     
+ Partials        211      210       -1

Impacted Files	Coverage Δ	Complexity Δ
...nix/components/toolbar/BrowserToolbarController.kt	62.2% <0%> (-2.33%)	0% <0%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2603683...1e0680e. Read the comment docs.

[flodolo]

@baron-severin
I'm having a hard time understanding this patch.

1. Why do you need to keep around the old strings? A "DEPRECATED" comment is not going to prevent 100 languages from translating them, and will waste time and resources.
2. Why are we deprecating more than just the Wi-Fi string? Does it mean that we need new strings to replace them? If that's the case, we should remove all these strings, and land them when they're ready. As far as I can tell, these strings landed without code behind in For 8017: add strings for advanced video autoplay #8911

[severinrudie]

@flodolo

Why do you need to keep around the old strings?

Our process has been to mark as deprecated then periodically go through and remove old strings. I don't remember what problem it was solving, tbh. I'll bring it up with my team since it sounds like it doesn't work well for you.

Why are we deprecating more than just the Wi-Fi string?

I forgot to deprecate a few while adding new strings in #8911.

Does it mean that we need new strings to replace them?

No, we should have everything we need here unless requirements change.

we should remove all these strings, and land them when they're ready.

Normally we would. This close to deadlines we've been avoiding adding any strings at all. Where they're absolutely necessary, we're landing them early (before code is ready) to give localizers as much time as possible.

[flodolo]

Why do you need to keep around the old strings?

Our process has been to mark as deprecated then periodically go through and remove old strings. I don't remember what problem it was solving, tbh. I'll bring it up with my team since it sounds like it doesn't work well for you.

That makes sense if you're already shipping a feature, and want to be able to ship more updates while the new strings are not used yet. At that point you definitely need the old (deprecated) strings in the repository.

As I said, these strings seem to have landed without code behind, so we're asking to localize twice the content for no good reason. Is that assumption correct?

Note that both deprecated and new strings haven't been exposed to localization yet, they're stuck in quarantine until I understand if we can avoid this.

[severinrudie]

preference_option_autoplay_allowed and preference_option_autoplay_blocked have been in the app since September of last year, they need to be deprecated following the normal approach. preference_option_autoplay_allowed_wifi_only has not yet been released, I duplicated it because I have been told to never change strings that have been committed to master. I don't know how the localization implementation details work, I can remove preference_option_autoplay_allowed_wifi_only if this is an exception to that rule.

If that looks good to you @flodolo, please take a look at #9068 and verify that we're set to move forward.

Current feature mock (available on release):

Mock for #8017 (the strings in this mock were provided by UX, and differ slightly from what we're landing):

[flodolo]

preference_option_autoplay_allowed and preference_option_autoplay_blocked have been in the app since September of last year, they need to be deprecated following the normal approach.

Thanks, I missed that.

preference_option_autoplay_allowed_wifi_only has not yet been released, I duplicated it because I have been told to never change strings that have been committed to master. I don't know how the localization implementation details work, I can remove preference_option_autoplay_allowed_wifi_only if this is an exception to that rule.

"Do not change strings in master" is the correct approach, because you don't know if a string is only in quarantine, or exposed for localization. In this case, it's stuck in quarantine and can be removed, or changed without a new ID.

[severinrudie]

👍 thanks for helping clear that up @flodolo