934274 - [app manager] validator.invalidLaunchPath=Unable to access to app starting document '%S': second "to" should be "the"

Status: RESOLVED FIXED

(DevTools Graveyard :: WebIDE, defect)

Description

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

/mozilla/browser/locales/en-US/chrome/browser/devtools/app-manager.properties

validator.invalidLaunchPath=Unable to access to app starting document '%S'
validator.invalidLaunchPathBadHttpCode=Unable to access to app starting document '%1$S', got HTTP code %2$S

The second "to"s should be "the"s. The entity IDs should be changed (e.g. by appending a 2), of course also where they are being called.

Comment 1

[cptanu]

I am ready to fix this bug. Please assign it to me.

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Great!  I've assigned the bug to you.  I believe comment 0 explains what's needed pretty clearly, but let me know if you have any questions.

Also, if you've never worked with the Firefox code base before, check out our Hacking[1] page for help with that.

[1]: https://wiki.mozilla.org/DevTools/Hacking

Comment 3

[cptanu]

Attachment: bug

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 827235 [details] [diff] [review]
bug

Review of attachment 827235 [details] [diff] [review]:
-----------------------------------------------------------------

Overall, looks like you've got the right idea!  Just a few naming improvements and reverting an unrelated file is needed.

Once you've made these changes, flag me for review on the updated patch.

::: browser/locales/en-US/chrome/browser/devtools/app-manager.properties
@@ +34,4 @@
>  validator.invalidHostedPriviledges=Hosted App can't be type '%S'.
>  validator.noCertifiedSupport='certified' apps are not fully supported on the App manager.
>  validator.nonAbsoluteLaunchPath=Launch path has to be an absolute path starting with '/': '%S'
> +validator.invalidLaunchPath1=Unable to access the app starting document '%S'

If you are going to add a number, then using "2" tends to be the convention, as in "invalidLaunchPath2".  The thinking is that the original is #1, so the fix would be #2.

Slightly rewording the key can work too, just to avoid the somewhat unsightly "2", which I think reads a bit better as well. :) Here we could go with "accessFailedLaunchPath", so something like that would be a good choice.

@@ +39,2 @@
>  # the launch document, %2$S is the http error code.
> +validator.invalidLaunchPathBadHttpCode1=Unable to access the app starting document '%1$S', got HTTP code %2$S

As above, we could use a "2", but let's make it "accessFailedLaunchPathBadHttpCode" to skip the number entirely.

::: dom/tests/mochitest/webapps/test_install_errors.xul
@@ +24,4 @@
>    invalidManifest,
>    permissionDenied,
>    invalidContent,
> +  invalidLaunchPath1,

This file is unrelated to what we are changing here, so please revert this change.

@@ +103,4 @@
>    };
>  }
>  
> +function invalidLaunchPath1(next) {

Revert this too.

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Also, you'll need to use proper Mercurial formatting on your patch, with a correctly formatted commit message.  See the guide[1] for more info.

If you are using Git, the same page has info about tools to make it generate patch in the right format.

[1]: https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

Comment 6

[cptanu]

Attachment: bug

Comment 7

[cptanu]

Sorry. I didn't see comment5. I will make the required changes and submit it again.

Comment 8

[cptanu]

Attachment: bug934274.patch

Comment 9

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 827281 [details] [diff] [review]
bug934274.patch

Review of attachment 827281 [details] [diff] [review]:
-----------------------------------------------------------------

Great!  This looks good to me.

I've pushed this change to our Try server to run tests on various platforms, which will take several hours to run:

https://tbpl.mozilla.org/?tree=Try&rev=6cb47afb0f52

Assuming the tests come back green, then you can add the keyword "checkin-needed" on this bug, and someone will land the patch for you.

Thanks again for your contribution! :D

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/7a2feb9904f0

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7a2feb9904f0