Last Comment Bug 724326 - [OpenSearch]: Fix unnecessary and unlocalizable "Search the web" string
: [OpenSearch]: Fix unnecessary and unlocalizable "Search the web" string
Status: RESOLVED FIXED
[good first bug][mentor=mkmelin][stri...
:
Product: Thunderbird
Classification: Client Software
Component: Search (show other bugs)
: 11 Branch
: All All
: -- normal (vote)
: Thunderbird 24.0
Assigned To: Suyash Agarwal (:sshagarwal)
:
Mentors:
http://hg.mozilla.org/comm-central/an...
Depends on: 677421
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-04 17:50 PST by Nomis101
Modified: 2013-06-25 05:17 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Screenshot (11.50 KB, image/jpeg)
2012-02-04 17:50 PST, Nomis101
no flags Details
Patch (1.18 KB, patch)
2013-05-19 08:17 PDT, Suyash Agarwal (:sshagarwal)
mkmelin+mozilla: review+
mkmelin+mozilla: feedback+
Details | Diff | Splinter Review

Description Nomis101 2012-02-04 17:50:47 PST
Created attachment 594504 [details]
Screenshot

"Search the web" is not localizable:
http://hg.mozilla.org/comm-central/file/77d2995562e0/mail/base/content/mailWindowOverlay.xul#l614
Which leads in some cases to a mixed rightclick menu.
Comment 1 Jim Porter (:squib) 2012-02-04 18:49:39 PST
This isn't actually an issue (except for overall cleanliness) since the label is always set here: http://mxr.mozilla.org/comm-central/source/mail/base/content/nsContextMenu.js#189
Comment 2 rsx11m 2012-02-04 20:56:42 PST
Yes, I've noticed that as well and figured that it's just a fallback if the string isn't defined otherwise. Do you see any cases where it actually shows up?
Comment 3 rsx11m 2012-02-04 20:59:38 PST
(hmm, judging from the screen shot apparently it /may/ show up...)

Those strings are defined in http://mxr.mozilla.org/comm-central/source/mail/locales/en-US/chrome/messenger/glodaComplete.properties#54
Comment 4 Jim Porter (:squib) 2012-02-04 21:25:03 PST
Nomis101, what did you right click on to get that popup? Nothing I've clicked on looks like that. I'm guessing the entries are "Paste", "Select All", "Check Spelling", "Add to Dictionary", and "Reply", but I can't get a context menu that looks anything like that.
Comment 5 Jim Porter (:squib) 2012-02-04 21:26:16 PST
Ohhh, wait, you're doing this from a standalone message window (in 11.0), right? In that case, this is a dupe of bug 720420.
Comment 6 Nomis101 2012-02-05 01:21:24 PST
(In reply to Jim Porter (:squib) from comment #5)
> Ohhh, wait, you're doing this from a standalone message window (in 11.0),
> right? In that case, this is a dupe of bug 720420.

Yes, I also hit bug 720420 in that contextual menu.

*** This bug has been marked as a duplicate of bug 720420 ***
Comment 7 Jim Porter (:squib) 2012-02-09 00:18:00 PST
Actually, let's undupe this and morph it into a bug for getting rid of that bogus unlocalized string entirely.
Comment 9 Suyash Agarwal (:sshagarwal) 2013-05-18 23:53:00 PDT
Sir,

I tried to reproduce the problem following the steps to reproduce as mentioned here:

https://bugzilla.mozilla.org/show_bug.cgi?id=720420#c0

But, I was unable to find "Search the web" option in the context menu. And, when I selected a piece of text, then the right-click context menu contained the option "Search Bing for:<selected_string>", but there was no option as "Search the web"

So, please tell me how to reproduce the error.
Comment 10 Magnus Melin 2013-05-19 03:28:12 PDT
Yeah the "Search the web" should never really show up anywhere, it's always replaced with a "Search Bing......" before showing - http://mxr.mozilla.org/comm-central/source/mail/base/content/nsContextMenu.js#166

Thinking about it, maybe it's better to just change it to label="[glodaComplete.webSearch1.label]" as a placeholder instead of removing it. That way it's easier to find out what's going on.
Comment 11 Suyash Agarwal (:sshagarwal) 2013-05-19 08:17:13 PDT
Created attachment 751473 [details] [diff] [review]
Patch

I have made the change.
Comment 12 Magnus Melin 2013-05-29 11:39:38 PDT
Comment on attachment 751473 [details] [diff] [review]
Patch

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

Looks good to me!
Comment 13 Magnus Melin 2013-05-29 12:08:12 PDT
Comment on attachment 751473 [details] [diff] [review]
Patch

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

Looks good to me!
Comment 14 Ryan VanderMeulen [:RyanVM] 2013-05-29 15:55:18 PDT
https://hg.mozilla.org/comm-central/rev/aed39342a55a

Note You need to log in before you can comment on or make changes to this bug.