395640 - "always ask" and "handle internally" options for feed entry in Applications prefpane should be renamed

Status: RESOLVED FIXED

(Firefox :: File Handling, defect, P2)

Description

[Myk Melez [:myk] [@mykmelez]]

faaborg's review in bug 377784, comment 45 says that the "always ask" option for the Web Feed entry in the Applications prefpane, which is currently called "Show me a preview and ask me what to do", should be labeled "Preview in Firefox", while the "handle internally" option, which is currently called "Live Bookmarks", should be called "Live Bookmarks in Firefox".

Comment 1

[Myk Melez [:myk] [@mykmelez]]

Err, un-ccing mconnor.  I don't want to spam him with a bunch of bug mail, so I'm only going to cc: him on the stuff he's commented on in bug 377784.

Comment 2

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v1: changes labels

Comment 3

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 280824 [details] [diff] [review]
patch v1: changes labels

Gavin pointed out on IRC that we should be using brandShortName instead of
hardcoding the name of the application in the properties.  My patch for bug 395644 adds the necessary plumbing to make that possible, so I'll submit an updated patch for this bug once the patch on that bug lands.

Comment 4

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v2: uses brandShortName

Here's a patch that uses brandShortName instead of hardcoding the name of the application in the properties file.  This patch also memoizes brandShortName so that retrieving it doesn't hurt performance for tasks that call describePreferredAction many times.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 281220 [details] [diff] [review]
patch v2: uses brandShortName

>Index: browser/components/preferences/applications.js

>@@ -1069,25 +1076,28 @@ var gApplicationsPane = {

>+      let label = this._prefsBundle.getFormattedString("alwaysAskAboutFeed",

>+      let label = this._prefsBundle.getFormattedString("liveBookmarks",

nit: unnecessarily redeclaring "label"

>Index: browser/locales/en-US/chrome/browser/preferences/preferences.properties

>+# LOCALIZATION NOTE (alwaysAskAboutFeed, liveBookmarks): %S = brandShortName
>+alwaysAskAboutFeed=Preview in %S
>+liveBookmarks=Live Bookmarks in %S

As I mentioned on IRC, it wouldn't hurt to change the entity names here to indicate a semantic change, even if it's fairly unlikely that localizers have started translating these yet.

Comment 6

[Myk Melez [:myk] [@mykmelez]]

Attachment: patch v3: review fixes

(In reply to comment #5)
> nit: unnecessarily redeclaring "label"

Urk, right, fixed.


> >+# LOCALIZATION NOTE (alwaysAskAboutFeed, liveBookmarks): %S = brandShortName
> >+alwaysAskAboutFeed=Preview in %S
> >+liveBookmarks=Live Bookmarks in %S
> 
> As I mentioned on IRC, it wouldn't hurt to change the entity names here to
> indicate a semantic change, even if it's fairly unlikely that localizers have
> started translating these yet.

Erm, right, how about previewInApp and liveBookmarksInApp?

Comment 7

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 281236 [details] [diff] [review]
patch v3: review fixes

Requesting approval to land this low-risk polish fix that renames two actions in the actions menu for the feed type (and refactors a bit of code in the process).

Comment 8

[Myk Melez [:myk] [@mykmelez]]

Checking in browser/components/preferences/applications.js;
/cvsroot/mozilla/browser/components/preferences/applications.js,v  <--  applications.js
new revision: 1.8; previous revision: 1.7
done
Checking in browser/locales/en-US/chrome/browser/preferences/preferences.properties;
/cvsroot/mozilla/browser/locales/en-US/chrome/browser/preferences/preferences.properties,v  <--  preferences.properties
new revision: 1.17; previous revision: 1.16
done