Hard-coded strings need to be replaced with entities

RESOLVED FIXED

Status

Mozilla Labs Graveyard
Test Pilot
RESOLVED FIXED
8 years ago
2 years ago

People

(Reporter: flod, Assigned: flod)

Tracking

Details

Attachments

(2 attachments, 1 obsolete attachment)

Comment hidden (empty)
(Assignee)

Comment 1

8 years ago
Sorry, the bug was submitted by mistake without a description (how is this possible?)

In the current version, there are hardcoded strings.

extension/content/all-studies-window.xul
"Data Submission" and "Notifications" are hardcoded

extension/content/feedback-browser.xul
menuitem id="feedback-menu-show-studies" has an hardcoded "..." in the label (label="&testpilot.settings.notifyMeWhen.label;...")

Extension's description should be also made localizable
https://developer.mozilla.org/en/localizing_extension_descriptions
Summary: Hard-coded strings should be made localizable → Hard-coded strings need to be replaced with entities
(Assignee)

Comment 2

8 years ago
Created attachment 462740 [details] [diff] [review]
Fix (partially or completely) hard-coded strings, add default pref to make description localizable

Since this bug has fallen out of radar for more than two weeks and lost the b3 train, let's try a self-made patch (not sure this is the right way, never worked before on more than one file).
Attachment #462740 - Flags: review?(jdicarlo)

Comment 3

8 years ago
Comment on attachment 462740 [details] [diff] [review]
Fix (partially or completely) hard-coded strings, add default pref to make description localizable

I'm not totally sold on the ellipsis use, but from a technical point of view, this is what a multi-file patch looks like :-)
(Assignee)

Comment 4

8 years ago
(In reply to comment #3)
> I'm not totally sold on the ellipsis use

Me neither. Besides, the current version uses the same "Notify me when" string twice, one with an hard-coded trailing "…" and one without.

Comment 5

8 years ago
Hi Flod,
Sorry for letting this go unfixed so long.  It wasn't really "fallen off the radar" so much as I've been overloaded with other stuff.

Thank you very, very much for writing this patch!  I really appreciate it.  The patch looks fine to me.  The unicode ellipsis character literals match what we're already using in main.properties (at the request of the l10n team!) so I think that's the right thing to do.

I imported the patch to the Test Pilot Hg repo in http://hg.mozilla.org/labs/testpilot/rev/343335b8ee8d .  It still needs to be approved for inclusion in Firefox 4 beta 4.
(Assignee)

Comment 6

8 years ago
(In reply to comment #5)
> It still needs to be approved for inclusion in Firefox 4 beta 4.

How does it work? Should I ask for approval to someone else? We're approaching the code freeze for b4 and I'd really like to see this fixed.

Comment 7

8 years ago
Hi Flod,
I've asked the Firefox team to review it.  You don't need to do anything else.
Thanks!
Comment on attachment 462740 [details] [diff] [review]
Fix (partially or completely) hard-coded strings, add default pref to make description localizable

Though the locale entities need to be aligned properly.
Attachment #462740 - Flags: review?(jdicarlo) → review+
Attachment #462740 - Flags: approval2.0+
(Assignee)

Comment 9

8 years ago
Created attachment 465782 [details] [diff] [review]
Patch with locale entities properly aligned

Dave, is this ok? I never realized that .dtd and .properties files use spaces instead of tabulations.
Created attachment 465818 [details] [diff] [review]
patch for mozilla-central
Assignee: nobody → francesco.lodolo

Comment 11

8 years ago
http://hg.mozilla.org/projects/electrolysis/rev/1a841e176c6c
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Assignee)

Updated

8 years ago
Attachment #465782 - Attachment is obsolete: true
Product: Mozilla Labs → Mozilla Labs Graveyard
You need to log in before you can comment on or make changes to this bug.