"New Non-e10s Window" is not localizable

RESOLVED FIXED in Firefox 41



3 years ago
3 years ago


(Reporter: flod, Assigned: dvedvick, Mentored)


Firefox 41

Firefox Tracking Flags

(e10s-, firefox41 fixed)


(Whiteboard: [good first bug][lang=js])


(2 attachments)



3 years ago

Bug 1163062 made strings localizable, but not this one. Not sure if there are others that I never noticed.
There is also "After restart, a tab will open to input.mozilla.org where you can provide us feedback about your e10s experience."


3 years ago
tracking-e10s: --- → -
Mentor: felipc
Whiteboard: [good first bug][lang=js]

Comment 3

3 years ago
I wouldn't mind taking a look at this if that's okay. It would be my first bug :)

It also looks like  "New Non-e10s Window" needs to be localized in CustomizableWidgets.jsm.
Yep, your help is very welcome! I have assigned the bug to you. Do you know how to edit the source code and compile it? Let me know if you have any questions
Assignee: nobody → dvedvick

Comment 5

3 years ago
Yes I do! Thanks!

Comment 6

3 years ago
Created attachment 8613311 [details] [diff] [review]
Make various e10s related string literals localizable

Comment 7

3 years ago
Felipe, I finally got through making the changes that I could find.
Comment on attachment 8613311 [details] [diff] [review]
Make various e10s related string literals localizable

Review of attachment 8613311 [details] [diff] [review]:

That's great, David! Very close to final. Comments below.

BTW, when you post a patch, you generally should already request feedback or review on that patch, as it is easier for someone to notice. Since I think there will be no more changes needed on the next patch, you can already request "review?" from me.

::: browser/components/preferences/in-content/main.js
@@ -366,5 @@
>                    .getAttribute("windowtype") == "navigator:browser") {
>        // We should only include visible & non-pinned tabs
>        tabs = win.gBrowser.visibleTabs.slice(win.gBrowser._numPinnedTabs);
> -      

your text editor is probably fixing some leftover whitespace on this file. That is generally a good thing to do, but since this is touching a part of the file that is unrelated to your patch, we generally avoid doing this kind of changes.

::: browser/locales/en-US/chrome/browser/preferences/preferences.properties
@@ +144,5 @@
>  featureEnableRequiresRestart=%S must restart to enable this feature.
>  featureDisableRequiresRestart=%S must restart to disable this feature.
>  shouldRestartTitle=Restart %S
> +
> +#### e10S

Please use the proper comment style as above. A Localization Note is very helpful for people who will do these translations, so you can write a very brief explanation about this string. I think the main important to mention is that this string will appear when the user unchecks the "enable multi-process" checkbox.
Attachment #8613311 - Flags: feedback+

Comment 9

3 years ago
Created attachment 8613847 [details] [diff] [review]
Implement code review changes

Hi Felipe, I think I implemented the changes. I was experimenting with the Atom editor for this, and it worked fine, but yes those whitespaces seem to be removed irregardless of what I did. So gedit fixed that!
Attachment #8613847 - Flags: review?(felipc)

Comment 10

3 years ago
Felipe - oops, it looks like Atom still attempted to fix some whitespaces in preferences.properties. Until I can configure Atom to *not* do that (or if it's a bug, when it is fixed) I will need to use an older, saner editor! Is it possible for me to edit existing patches, and re-upload them? What's the proper process for re-work?
Comment on attachment 8613847 [details] [diff] [review]
Implement code review changes

Hi David, thanks for the update! About the whitespace changes done by Atom, that's no problem for your first patch! I'll adjust that when generating the commit for your patch.
Attachment #8613847 - Flags: review?(felipc) → review+
David, I see that your author name in the patch is:

david <dvedvick@...>

do you want to use only "david" like that or do you want to change that for the proper commit?
Flags: needinfo?(dvedvick)

Comment 13

3 years ago
Felipe, I see your point. Use "vedvick", that's a fairly unique name. Thanks.
Flags: needinfo?(dvedvick)
Last Resolved: 3 years ago
status-firefox41: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
You need to log in before you can comment on or make changes to this bug.