1408369 - [dt-onboarding] Move aboutdevtools.dtd back to locales/en-US folder when ready for localization

Status: RESOLVED FIXED

(DevTools :: General, enhancement, P3)

Description

[Julian Descottes [:jdescottes]]

Follow up to Bug 1408368.

We are temporarily moving aboutdevtools.dtd out of devtools/shim/locales/en-US because the file is not ready for localization. We need to revert this change and comment the DTD file properly when the strings are ready to be localized.

Comment 1

[Julian Descottes [:jdescottes]]

Attachment: Bug 1408369 - create about:devtools welcome message dynamically;

Review commit: https://reviewboard.mozilla.org/r/196654/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/196654/

Comment 2

[Julian Descottes [:jdescottes]]

Attachment: Bug 1408369 - move back aboutdevtools.dtd to devtools/shim/locales;

Review commit: https://reviewboard.mozilla.org/r/196656/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/196656/

Comment 3

[Francesco Lodolo [:flod]]

Comment on attachment 8925519 [details]
Bug 1408369 - create about:devtools welcome message dynamically;

https://reviewboard.mozilla.org/r/196654/#review201924

General question: is it OK to have all those "Firefox" hard-coded, instead of a brand name that can change based on the build? My guess would be yes, but better to ask.

::: devtools/shim/aboutdevtools/aboutdevtools.xhtml:61
(Diff revision 1)
>          <!-- Form dedicated to the newsletter subscription -->
>          <div class="newsletter">
>            <h2 class="newsletter-title">&aboutDevtools.newsletter.title;</h2>
>            <p>&aboutDevtools.newsletter.message;</p>
>  
>            <form id="newsletter-form" name="newsletter-form" action="https://www.mozilla.org/en-US/newsletter/" method="post">

Drop en-US from links to mozilla.org, let the website do the proper redirection based on the user's preferences.

::: devtools/shim/aboutdevtools/aboutdevtools.xhtml:101
(Diff revision 1)
>             alt="Firefox Developer Edition logo"/>
>        <div class="footer-message">
> -        <h1 class="footer-message-title">Firefox Developer Edition</h1>
> -        <p>
> -          Looking for more than just Developer Tools?
> -          Check out the Firefox browser that is built specifically for developers and modern workflows.
> +        <h1 class="footer-message-title">&aboutDevtools.footer.title;</h1>
> +        <p>&aboutDevtools.footer.message;</p>
> +        <a class="external footer-link"
> +           href="https://www.mozilla.org/en-US/firefox/developer/"

Same here, remove /en-US/

::: devtools/shim/aboutdevtools/tmp-locale/aboutdevtools.dtd:72
(Diff revision 1)
>  <!ENTITY  aboutDevtools.newsletter.thanks.message "If you haven’t previously confirmed a subscription to a Mozilla-related newsletter you may have to do so. Please check your inbox or your spam filter for an email from us.">
>  
> +<!ENTITY  aboutDevtools.footer.title "Firefox Developer Edition">
> +<!ENTITY  aboutDevtools.footer.message "Looking for more than just Developer Tools? Check out the Firefox browser that is built specifically for developers and modern workflows.">
> +
> +<!-- LOCALIZATION NOTE (aboutDevtools.enable.learnMoreLink): Text for the link to

Wront string reference in the comment

::: devtools/shim/aboutdevtools/tmp-locale/aboutdevtools.dtd:73
(Diff revision 1)
>  
> +<!ENTITY  aboutDevtools.footer.title "Firefox Developer Edition">
> +<!ENTITY  aboutDevtools.footer.message "Looking for more than just Developer Tools? Check out the Firefox browser that is built specifically for developers and modern workflows.">
> +
> +<!-- LOCALIZATION NOTE (aboutDevtools.enable.learnMoreLink): Text for the link to
> +  -  https://www.mozilla.org/en-US/firefox/developer/ displayed in the footer. -->

Remove en-US, even if it's just a comment

Comment 4

[Julian Descottes [:jdescottes]]

Thanks for jumping on this :flod, I'll update the patch with your comments before sending it back to you for a final review!

Comment 5

[Julian Descottes [:jdescottes]]

Comment on attachment 8925519 [details]
Bug 1408369 - create about:devtools welcome message dynamically;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/196654/diff/1-2/

Comment 6

[Julian Descottes [:jdescottes]]

Comment on attachment 8925520 [details]
Bug 1408369 - move back aboutdevtools.dtd to devtools/shim/locales;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/196656/diff/1-2/

Comment 7

[Julian Descottes [:jdescottes]]

Comment on attachment 8925519 [details]
Bug 1408369 - create about:devtools welcome message dynamically;

https://reviewboard.mozilla.org/r/196654/#review201924

> Drop en-US from links to mozilla.org, let the website do the proper redirection based on the user's preferences.

For this particular link the en-US doesn't have any impact on the actual action performed on the server for POST requests. Newsletter localization will be handled in Bug 1415273 (basically we need to let the user pick one of the available languages for the newsletter)

Comment 8

[Julian Descottes [:jdescottes]]

(In reply to Julian Descottes [:jdescottes] from comment #7)
> Comment on attachment 8925519 [details]
> Bug 1408369 - create about:devtools welcome message dynamically;
> 
> https://reviewboard.mozilla.org/r/196654/#review201924
> 
> > Drop en-US from links to mozilla.org, let the website do the proper redirection based on the user's preferences.
> 
> For this particular link the en-US doesn't have any impact on the actual
> action performed on the server for POST requests. Newsletter localization
> will be handled in Bug 1415273 (basically we need to let the user pick one
> of the available languages for the newsletter)

This first comment was about the en-US in the following form action attribute.

<form id="newsletter-form" 
      name="newsletter-form" 
      action="https://www.mozilla.org/en-US/newsletter/" 
      method="post">

Comment 9

[Francesco Lodolo [:flod]]

(In reply to Julian Descottes [:jdescottes] from comment #7)
> For this particular link the en-US doesn't have any impact on the actual
> action performed on the server for POST requests. Newsletter localization
> will be handled in Bug 1415273 (basically we need to let the user pick one
> of the available languages for the newsletter)

Sorry but I'm not following. By using https://www.mozilla.org/en-US/newsletter/, I assume that you send all users to a confirmation page in English, instead of sending them to a localized page. That is unrelated to the language of the newsletter itself.

Example: I'm on an Italian browser, if I open https://www.mozilla.org/newsletter/ I get to https://www.mozilla.org/it/newsletter/ (localized), and get a confirmation message in Italian if I suscribe. Note that newsletter are not localized in Italian at all, only 6 languages are available.

I really think that URL should be https://www.mozilla.org/newsletter/, not https://www.mozilla.org/en-US/newsletter/

Comment 10

[Julian Descottes [:jdescottes]]

(In reply to Francesco Lodolo [:flod] from comment #9)
> (In reply to Julian Descottes [:jdescottes] from comment #7)
> > For this particular link the en-US doesn't have any impact on the actual
> > action performed on the server for POST requests. Newsletter localization
> > will be handled in Bug 1415273 (basically we need to let the user pick one
> > of the available languages for the newsletter)
> 
> Sorry but I'm not following. By using
> https://www.mozilla.org/en-US/newsletter/, I assume that you send all users
> to a confirmation page in English, instead of sending them to a localized
> page. That is unrelated to the language of the newsletter itself.
> 
> Example: I'm on an Italian browser, if I open
> https://www.mozilla.org/newsletter/ I get to
> https://www.mozilla.org/it/newsletter/ (localized), and get a confirmation
> message in Italian if I suscribe. Note that newsletter are not localized in
> Italian at all, only 6 languages are available.
> 
> I really think that URL should be https://www.mozilla.org/newsletter/, not
> https://www.mozilla.org/en-US/newsletter/

We are not sending the users to the page, it's just the URL we use to send the subscription request via a XHR. The confirmation message is handled in about:devtools. No navigation involved, we preventDefault() on the submit event of the form.

Comment 11

[Francesco Lodolo [:flod]]

(In reply to Julian Descottes [:jdescottes] from comment #10)
> We are not sending the users to the page, it's just the URL we use to send
> the subscription request via a XHR. The confirmation message is handled in
> about:devtools. No navigation involved, we preventDefault() on the submit
> event of the form.

Thanks, that was the part that I was missing.

Comment 12

[Francesco Lodolo [:flod]]

Comment on attachment 8925519 [details]
Bug 1408369 - create about:devtools welcome message dynamically;

https://reviewboard.mozilla.org/r/196654/#review202830

Side question: can "DevTools" be localized? We're localizing "Developer Tools", and we only have 3 strings using "DevTools" in m-c, while these patches add a lot of them.

Comment 13

[Francesco Lodolo [:flod]]

Comment on attachment 8925520 [details]
Bug 1408369 - move back aboutdevtools.dtd to devtools/shim/locales;

https://reviewboard.mozilla.org/r/196656/#review202832

Mostly nits and questions, only one real thing that we should fix.

::: devtools/shim/locales/en-US/aboutdevtools.dtd:28
(Diff revision 2)
> +  -  the Inspect Element menu item. -->
>  <!ENTITY  aboutDevtools.enable.inspectElementTitle "Enable Firefox Developer Tools to use Inspect Element">
>  
> +<!-- LOCALIZATION NOTE (aboutDevtools.enable.aboutDebuggingMessage): Message displayed
> +  -  when users come from about:debugging. -->
>  <!ENTITY  aboutDevtools.enable.aboutDebuggingMessage

All messages but inspectElement don't have an associated title?

::: devtools/shim/locales/en-US/aboutdevtools.dtd:33
(Diff revision 2)
>  <!ENTITY  aboutDevtools.enable.aboutDebuggingMessage
>            "Develop and debug WebExtensions, web workers, service workers and more with Firefox DevTools.">
> +
> +<!-- LOCALIZATION NOTE (aboutDevtools.enable.inspectElementMessage): Message displayed
> +  -  when users come from using the Inspect Element menu item. -->
>  <!ENTITY  aboutDevtools.enable.inspectElementMessage

We should move this near the aboutDevtools.enable.inspectElementTitle

::: devtools/shim/locales/en-US/aboutdevtools.dtd:47
(Diff revision 2)
> +  -  clicked on a "Enable Developer Tools" menu item. -->
>  <!ENTITY  aboutDevtools.enable.menuMessage
>            "Examine, edit and debug HTML, CSS, and JavaScript with tools like Inspector and Debugger.">
>  
> +<!-- LOCALIZATION NOTE (aboutDevtools.enable.commonMessage): Generic message displayed for
> +  -  all possible entry points (keyshortcut, menu item etc...). -->

Uber nit-pick: single unicode … instead of ...

::: devtools/shim/locales/en-US/aboutdevtools.dtd:67
(Diff revision 2)
> +<!-- LOCALIZATION NOTE (aboutDevtools.newsletter.message): Subscribe form message.
> +  -  The newsletter is only available in english at the moment.
> +  -  See Bug 1415273 for support of additional languages.-->
>  <!ENTITY  aboutDevtools.newsletter.message "Get developer news, tricks and resources sent straight to your inbox.">
>  <!ENTITY  aboutDevtools.newsletter.email.placeholder "Email">
>  <!ENTITY  aboutDevtools.newsletter.privacy.label "I’m okay with Mozilla handling my info as explained in this <a class=&#x0022;external&#x0022; href=&#x0022;https://www.mozilla.org/privacy/&#x0022;>Privacy Policy</a>.">

Can we avoid using &#x0022 here? It's quite horrible, and easy to break.

We have plenty of HTML fragments in string, they use a single straight quote, e.g. https://hg.mozilla.org/l10n/gecko-strings/file/default/browser/chrome/overrides/netError.dtd#l152

You'll only need to add that string to this test's exceptions
http://searchfox.org/mozilla-central/rev/ed212c79cfe86357e9a5740082b9364e7f6e526f/browser/base/content/test/static/browser_misused_characters_in_strings.js#17

Comment 14

[Julian Descottes [:jdescottes]]

(In reply to Francesco Lodolo [:flod] from comment #12)
> Comment on attachment 8925519 [details]
> Bug 1408369 - create about:devtools welcome message dynamically;
> 
> https://reviewboard.mozilla.org/r/196654/#review202830
> 
> Side question: can "DevTools" be localized? We're localizing "Developer
> Tools", and we only have 3 strings using "DevTools" in m-c, while these
> patches add a lot of them.

Good question, I think most of the time, it gets expanded to "Developer Tools" and then translated. For instance in french I found DevTools localized to "outils de développement". So I think we should translate them but I also think we should avoid them as much as we can since we don't have a good equivalent in other languages. 

Let's wait for a bit, I'll try to see if we can get rid of some of the "DevTools". Either expand to Developer Tools or remove it if the context is clear.

Comment 15

[Julian Descottes [:jdescottes]]

Comment on attachment 8925519 [details]
Bug 1408369 - create about:devtools welcome message dynamically;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/196654/diff/2-3/

Comment 16

[Julian Descottes [:jdescottes]]

Comment on attachment 8925520 [details]
Bug 1408369 - move back aboutdevtools.dtd to devtools/shim/locales;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/196656/diff/2-3/

Comment 17

[Julian Descottes [:jdescottes]]

Comment on attachment 8925520 [details]
Bug 1408369 - move back aboutdevtools.dtd to devtools/shim/locales;

https://reviewboard.mozilla.org/r/196656/#review202832

> Can we avoid using &#x0022 here? It's quite horrible, and easy to break.
> 
> We have plenty of HTML fragments in string, they use a single straight quote, e.g. https://hg.mozilla.org/l10n/gecko-strings/file/default/browser/chrome/overrides/netError.dtd#l152
> 
> You'll only need to add that string to this test's exceptions
> http://searchfox.org/mozilla-central/rev/ed212c79cfe86357e9a5740082b9364e7f6e526f/browser/base/content/test/static/browser_misused_characters_in_strings.js#17

Good point, done. Let me know if you are not comfortable reviewing the test change.

Comment 18

[Julian Descottes [:jdescottes]]

Comment on attachment 8925520 [details]
Bug 1408369 - move back aboutdevtools.dtd to devtools/shim/locales;

https://reviewboard.mozilla.org/r/196656/#review202832

> All messages but inspectElement don't have an associated title?

Yes, that is the only entry point with a custom title at the moment.

Comment 19

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 8925519 [details]
Bug 1408369 - create about:devtools welcome message dynamically;

https://reviewboard.mozilla.org/r/196654/#review202862

LGTM

::: devtools/shim/aboutdevtools/aboutdevtools.xhtml:53
(Diff revision 3)
>      <div class="box">
>        <div class="left-pane" />
>        <div class="right-pane">
>          <h1 class="title" >&aboutDevtools.welcome.title;</h1>
> -        <p id="welcome-message">&aboutDevtools.welcome.message;</p>
> +        <!-- The welcome message is dynamically updated with a keyboard shortcut at
> +             runtime and is added in aboutdevtools.js -->

nit: s/is added/added

Comment 20

[Julian Descottes [:jdescottes]]

Comment on attachment 8925519 [details]
Bug 1408369 - create about:devtools welcome message dynamically;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/196654/diff/3-4/

Comment 21

[Julian Descottes [:jdescottes]]

Comment on attachment 8925520 [details]
Bug 1408369 - move back aboutdevtools.dtd to devtools/shim/locales;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/196656/diff/3-4/

Comment 22

[Julian Descottes [:jdescottes]]

Comment on attachment 8925519 [details]
Bug 1408369 - create about:devtools welcome message dynamically;

https://reviewboard.mozilla.org/r/196654/#review202862

Thanks for the review, fixed the nit.

Comment 23

[Francesco Lodolo [:flod]]

Comment on attachment 8925520 [details]
Bug 1408369 - move back aboutdevtools.dtd to devtools/shim/locales;

https://reviewboard.mozilla.org/r/196656/#review202870

One final thing to fix, but it's look good to me, and seems simple enough to review.

::: devtools/shim/locales/en-US/aboutdevtools.dtd:29
(Diff revision 4)
>  <!ENTITY  aboutDevtools.enable.inspectElementTitle "Enable Firefox Developer Tools to use Inspect Element">
>  
> -<!ENTITY  aboutDevtools.enable.aboutDebuggingMessage
> -          "Develop and debug WebExtensions, web workers, service workers and more with Firefox DevTools.">
> +<!-- LOCALIZATION NOTE (aboutDevtools.enable.inspectElementMessage): Message displayed
> +  -  when users come from using the Inspect Element menu item. -->
>  <!ENTITY  aboutDevtools.enable.inspectElementMessage
> -          "Examine and edit HTML and CSS with the DevTools Inspector.">
> +          "Examine and edit HTML and CSS with the Developer Tools' Inspector.">

You need to use an apostrophe here, otherwise tests will fail.

Comment 24

[Julian Descottes [:jdescottes]]

Good catch, thanks!

Comment 25

[Julian Descottes [:jdescottes]]

Comment on attachment 8925520 [details]
Bug 1408369 - move back aboutdevtools.dtd to devtools/shim/locales;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/196656/diff/4-5/

Comment 26

[Julian Descottes [:jdescottes]]

try https://treeherder.mozilla.org/#/jobs?repo=try&revision=b2a607c50175261a0335d4e1b1112231d0cecf40

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/43a6a070e9d3
create about:devtools welcome message dynamically;r=flod,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/4cd32018fcd2
move back aboutdevtools.dtd to devtools/shim/locales;r=flod

Comment 28

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/43a6a070e9d3
https://hg.mozilla.org/mozilla-central/rev/4cd32018fcd2