Search filtering options aren't localizable

VERIFIED FIXED in mozilla0.9

Status

SeaMonkey
Search
P1
normal
VERIFIED FIXED
18 years ago
10 years ago

People

(Reporter: Blake Ross, Assigned: Blake Ross)

Tracking

({l12y})

Trunk
mozilla0.9

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments)

(Assignee)

Description

18 years ago
The items in the context menu that appears when you right click on a set of 
search results in the sidebar are not localizable, but rather are hardcoded in 
C++ (why this is even in C++ and not JS & dtd files is beyond me..)

http://lxr.mozilla.org/seamonkey/source/xpfe/components/search/src/nsInternetSea
rchService.cpp#1046

Comment 1

18 years ago
> why this is even in C++ and not JS & dtd files is beyond me..

Context menu commands are RDF-aggregatable and are implemented in C++ so that 
along with appearing in the search sidebar panel (in this instance) they also, 
for example, appear in the Bookmarks window/sidebar when appropriate.

Comment 2

18 years ago
adding localizability keyword
Keywords: l12y

Comment 3

18 years ago
adding nsbeta1 keyword to localizability bugs
Keywords: nsbeta1
(Assignee)

Comment 4

18 years ago
*** Bug 63112 has been marked as a duplicate of this bug. ***

Updated

18 years ago
Keywords: mozilla1.0
Netscape Nav triage team: this is a Netscape beta stopper.
Keywords: intl
Priority: P3 → P1

Updated

18 years ago
Keywords: intl

Updated

18 years ago
Target Milestone: --- → mozilla0.8
matts on it. 

Updated

18 years ago
Target Milestone: mozilla0.8 → mozilla0.9
(Assignee)

Updated

18 years ago
Assignee: matt → blakeross
(Assignee)

Comment 7

18 years ago
Matt, I have a fix for this.  Taking if you don't mind...
(Assignee)

Comment 8

18 years ago
*** Bug 67828 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 9

18 years ago
*** Bug 67909 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 10

18 years ago
Created attachment 25539 [details] [diff] [review]
[patch] localize values

Comment 11

18 years ago
Instead of:

+static NS_DEFINE_CID(kStringBundleServiceCID,      NS_STRINGBUNDLESERVICE_CID);

and

+    NS_WITH_SERVICE(nsIStringBundleService, stringService,
kStringBundleServiceCID, &rv);

use:

nsCOMPtr<nsIStringBundleService>
stringService(do_GetService(NS_STRINGBUNDLE_CONTRACTID, &rv));

+      nsCOMPtr<nsILocale> locale = nsnull;
+      nsCOMPtr<nsIStringBundle> bundle = nsnull;

No need to do the explicit initialization. Also, no need for the |locale|, see
below.

+      rv = stringService->CreateBundle(SEARCH_PROPERTIES, locale,
getter_AddRefs(bundle));

Because here you can do s/locale/nsnull/

+        if (source == kNC_SearchCommand_AddToBookmarks)
+          name.AssignWithConversion("addtobookmarks");

Use |name = NS_LITERAL_STRING("addToBookmarks");| (I thought property
identifiers were iCaps in properties files? Not sure though...). Use |.Assign()|
if you don't like the |=| :-)

+        if (NS_SUCCEEDED(rv) && !nsString(valUni).IsEmpty()) {

Instead of wrapping valUni in an nsString and using IsEmpty() it'd be cheaper to
do something like:

  if (NS_SUCCEEDED(rv) && valUni && *valUni)

(Yeah, I recall this conversation, and I've changed my mind :-) ).

+          if (NS_FAILED(rv =
gRDFService->GetLiteral(nsString(valUni).GetUnicode(),
getter_AddRefs(literal))))

Here you don't need the |nsString| + |.GetUnicode()|. A |const PRUnichar*| is
expected, which valUni will give.

The rest looks fine, though I suggest you remove the space between the var
declaration and the copy initialisation parenthesis to at least make it look
like a constructor call. Or rather, just leave those lines alone, since it's
just a style issue (see
<http://ScottCollins.net/programming/cpp/initialization.html>).
(Assignee)

Comment 12

18 years ago
Created attachment 25562 [details] [diff] [review]
[patch] with suggestions

Comment 13

18 years ago
r=timeless
w/ permission to simplify:
+    if (NS_SUCCEEDED(rv) && stringService) {
(Assignee)

Comment 14

18 years ago
Cc'ing erik for sr.

Comment 15

18 years ago
sr=erik

Next time, please separate the parts that are not relevant to this bug
(i.e. all the nsCOMPtr constructors).
(Assignee)

Comment 16

18 years ago
Fix checked in.
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
mass-verifying claudius' Fixed bugs which haven't changed since 2001.12.31.

if you think this particular bug is not fixed, please make sure of the following
before reopening:

a. retest with a *recent* trunk build.
b. query bugzilla to see if there's an existing, open bug (new, reopened,
assigned) that covers your issue.
c. if this does need to be reopened, make sure there are specific steps to
reproduce (unless already provided and up-to-date).

thanks!

[set your search string in mail to "AmbassadorKoshNaranek" to filter out these
messages.]
Status: RESOLVED → VERIFIED
Product: Core → SeaMonkey
You need to log in before you can comment on or make changes to this bug.