Opened 20 months ago

Closed 16 months ago

Last modified 14 months ago

#6633 closed change (fixed)

Move translatable strings from adblockpluschrome to adblockplusui

Reported by: greiner Assignee: saroyanm
Priority: P3 Milestone:
Module: User-Interface Keywords:
Cc: agiammarchi, saroyanm, wspee, kzar, sebastian, Shikitita Blocked By:
Blocking: #6845, #6892 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://gitlab.com/eyeo/adblockplus/abpui/adblockplusui/merge_requests/85
https://gitlab.com/eyeo/adblockplus/abpui/adblockplusui/merge_requests/86

Description (last modified by greiner)

Background

While we are planning to move all remaining UIs in adblockpluschrome to adblockplusui (see #6321 and #6323) the Platform team suggested to instead remove them when working on the next redesign.

However, at this point it is still unclear when that would happen so to avoid duplicating our translation process until then, we should move all UI-related strings from adblockpluschrome to adblockplusui.

That means that adblockpluschrome would be left with the strings "name" and "name_devbuild" so it may make sense to move them too.

What to change

  • Move adblockpluschrome/_locales/*/messages.json files to adblockplusui/locale/*/.
  • Split messages.json files up into dedicated files depending on which UI they are used for (i.e. common, composer, notification, popup, etc.).

See also https://gitlab.com/eyeo/adblockplus/adblockplusui/issues/154

Notes for dependency update

  • Remove all translatable strings from adblockpluschrome.
  • Import all translatable strings from the following new files from adblockplusui:
    • composer.json
    • filter-validation.json
    • manifest.json
    • notification-helper.json

Change History (20)

comment:1 follow-up: Changed 20 months ago by sebastian

Even with all UI components finally moved into adblockplusui, there will be more strings left than just the extension's name (and short description), e.g there are also filter invalidation errors which are translated in the adblockpluschrome based on IDs that come from adblockpluscore.

Still I agree that it would make sense to have a single translation project. FWIW, a related idea that has been around for a while is merging adblockplusui and adblockpluschrome into a single repository. Ideally, I like to see that happen and then combine the translations as part of that effort.

comment:2 in reply to: ↑ 1 Changed 20 months ago by greiner

Replying to sebastian:

Even with all UI components finally moved into adblockplusui, there will be more strings left than just the extension's name (and short description), e.g there are also filter invalidation errors which are translated in the adblockpluschrome based on IDs that come from adblockpluscore.

Are those used anywhere outside the UI? Because I was thinking not to resolve the strings in adblockpluschrome anymore but instead only pass an error ID to the UI which it can then translate into error messages. That'd be consistent with how we do it with filter list download errors (e.g. "synchronize_invalid_data").

FWIW, a related idea that has been around for a while is merging adblockplusui and adblockpluschrome into a single repository.

@wspee IIRC we were planning to set up a meeting to discuss the pros and cons that have been brought up around that.

Ideally, I like to see that happen and then combine the translations as part of that effort.

It's unclear when such a change would happen and how long it would take. Therefore I'd prefer going forward with this ticket independently so that we can already include those strings in our translation process.

comment:3 Changed 17 months ago by greiner

This topic has come up again so I'd like to move this forward. Just for reference, here are all of the strings that are currently in adblockpluschrome and what they're being used for:

18 strings directly related to adblockplusui

  • composer
    • add
    • add_filters_msg
    • loading
  • issue-reporter
    • cancel
    • continue
    • filters_subscription_lastDownload_connectionError
  • popup
    • clickhide_instructions
    • disabled_for_site
    • easy_create_filter
    • enabled_for_site
    • options_short
    • overlay_notification_closing_button_hide
    • overlay_notification_closing_button_optout
    • sendReport
    • stats_label_page
    • stats_label_total
    • stats_show_iconnumber
    • stats_title

9 strings closely related to adblockplusui

  • lib/filterComposer
    • block_element
  • lib/filterValidation: We could update FilterParsingError.toString() to return a serialized version of the error
    • filter_elemhideemulation_nodomain
    • filter_invalid_csp
    • filter_invalid_domain
    • filter_invalid_regexp
    • filter_unknown_option
    • invalid_css_selector
    • line
    • unexpected_filter_list_header

9 strings not closely related to adblockplusui

  • manifest.json
    • description
    • name
    • name_devbuild
  • lib/notificationHelper
    • notification_configure
    • notification_open_all
    • notification_with_buttons
    • notification_without_buttons
    • overlay_notification_button_no
    • overlay_notification_button_yes

Given that, I'd argue that it'd be more beneficial to also move the few remaining strings that are used in the manifest and in notifications for the sake of streamlining the translation process.

Last edited 17 months ago by greiner (previous) (diff)

comment:4 Changed 16 months ago by kzar

  • Blocking 6845 added

comment:5 Changed 16 months ago by kzar

OK, we have agreement on this now. You can go ahead and start adding those strings. I've created #6845 for the adblockpluschrome change.

comment:6 Changed 16 months ago by greiner

  • Description modified (diff)

comment:7 follow-up: Changed 16 months ago by saroyanm

Are those strings still being used anywhere ?

  • invalid_css_selector
  • line
  • filters_subscription_lastDownload_connectionError
  • unexpected_filter_list_header

I couldn't find any code that associates with those string. Would be great if anyone can confirm that those are still being used or not.

Maybe @greiner as you already had a look into them.

comment:8 Changed 16 months ago by saroyanm

comment:9 in reply to: ↑ 7 Changed 16 months ago by greiner

Replying to saroyanm:

Are those strings still being used anywhere ?

Yes, they are. They're just not easy to find due to the way the code around them is written.

comment:10 Changed 16 months ago by saroyanm

  • Priority changed from Unknown to P3
  • Ready set

comment:11 Changed 16 months ago by saroyanm

  • Owner set to saroyanm

comment:12 Changed 16 months ago by saroyanm

  • Review URL(s) modified (diff)

comment:13 Changed 16 months ago by saroyanm

  • Status changed from new to reviewing

comment:15 Changed 16 months ago by greiner

  • Description modified (diff)

comment:16 Changed 16 months ago by greiner

  • Blocking 6892 added

comment:17 Changed 16 months ago by greiner

  • Blocking 6892 removed

comment:18 Changed 16 months ago by greiner

  • Blocking 6892 added

comment:20 Changed 14 months ago by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

This is done and I have not noticed any string or translation related issues in 3.4 so far.

ABP 3.3.2.2175
Firefox 62 / 51 / Windows 10
Chrome 69 / 49 / Windows 10
Opera 56 / 36 / Windows 10

Note: See TracTickets for help on using tickets.