Last Comment Bug 846312 - Twitter OpenSearch appears to be busted
: Twitter OpenSearch appears to be busted
Status: RESOLVED FIXED
[good first bug][mentor=mkmelin][stri...
:
Product: Thunderbird
Classification: Client Software
Component: Search (show other bugs)
: Trunk
: All All
-- normal with 1 vote (vote)
: Thunderbird 24.0
Assigned To: Ekanan Ketunuti
:
:
Mentors:
http://mxr.mozilla.org/comm-central/s...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-02-28 06:51 PST by Mike Conley (:mconley)
Modified: 2013-06-25 05:18 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
proposed patch (1.44 KB, patch)
2013-05-21 21:34 PDT, Ekanan Ketunuti
no flags Details | Diff | Splinter Review
proposed patch, v2 (1.50 KB, patch)
2013-05-23 04:20 PDT, Ekanan Ketunuti
mkmelin+mozilla: review+
Details | Diff | Splinter Review

Description User image Mike Conley (:mconley) 2013-02-28 06:51:22 PST
STR:

1) In some message, select some text, and choose "Search X search for..." to open an OpenSearch tab
2) In the new tab, choose "Twitter"

What happens?

I get a "Sorry, that page doesn't exist!" error from Twitter.

What's expected?

Twitter search.
Comment 1 User image Oli 2013-05-15 08:25:44 PDT
Easy fix imo.

Thunderbird currently uses this URL:
"http://search.twitter.com/search?q=[searchterm]"

Whereas this is the correct one (at least it works for me):
"https://twitter.com/search?q=[searchterm]"

(Notice that Twitter always uses HTTPS since some time.)
Comment 2 User image Magnus Melin 2013-05-15 11:39:51 PDT
Do you want to take on creating a patch?
Comment 4 User image Magnus Melin 2013-05-18 04:01:17 PDT
Grr, bugzilla autolinks badly. But see the URL field
Comment 5 User image Ekanan Ketunuti 2013-05-21 21:34:14 PDT
Created attachment 752524 [details] [diff] [review]
proposed patch

fix the url and to keep consistent with other engines. i moved the param to element.
Comment 6 User image Magnus Melin 2013-05-22 12:27:08 PDT
Comment on attachment 752524 [details] [diff] [review]
proposed patch

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

::: mail/locales/en-US/searchplugins/twitter.xml
@@ +6,5 @@
>  <ShortName>Twitter Search</ShortName>
>  <Description>Realtime Twitter Search</Description>
>  <InputEncoding>UTF-8</InputEncoding>
>  <Image width="16" height="16">data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8/9hAAAAiklEQVR42mNgGDxgzTsBnOJr3hUAcQMQJyCJCaArXA9XgCp+Hoj/I+H9QHwfUy1EAqRgPtz0Ne8M0DTD8HuwHJoBCWgKQAb14zAgAJtf9+NQjIlxBJYB1GZCBpzHFxMNRBiSgM8AByyhTqTtqF5Zj1UzrrQCTRjz8Th/Pm7NmLY3IGFQ1CowDG8AAK5P9364gzTrAAAAAElFTkSuQmCC</Image>
> +<Url type="text/html" method="get" template="https://twitter.com/search">

please make it uppercase GET while you're at it

@@ +13,1 @@
>  <SearchForm>http://search.twitter.com/</SearchForm>

you forgot to change this
Comment 7 User image Magnus Melin 2013-05-22 12:28:54 PDT
For reference, firefox fixed this in bug 803335.
Comment 8 User image Magnus Melin 2013-05-22 12:30:39 PDT
I wonder if we should be sending a partner param, like firefox http://mxr.mozilla.org/comm-central/source/mozilla/browser/locales/en-US/searchplugins/twitter.xml#14
Comment 9 User image Mark Banner (:standard8) 2013-05-23 03:34:00 PDT
(In reply to Magnus Melin from comment #8)
> I wonder if we should be sending a partner param, like firefox
> http://mxr.mozilla.org/comm-central/source/mozilla/browser/locales/en-US/
> searchplugins/twitter.xml#14

No we shouldn't do (the arrangements for different apps are typically different and usually require different params if we do have them so typically we shouldn't sync these - any other data is fine).
Comment 10 User image Ekanan Ketunuti 2013-05-23 04:20:24 PDT
Created attachment 753219 [details] [diff] [review]
proposed patch, v2

addressed review comments.
Comment 11 User image Magnus Melin 2013-05-23 11:23:57 PDT
Comment on attachment 753219 [details] [diff] [review]
proposed patch, v2

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

Great! Thx for the patch. r=mkmelin
Comment 12 User image Ryan VanderMeulen [:RyanVM] 2013-05-28 10:17:50 PDT
https://hg.mozilla.org/comm-central/rev/809ded80b319

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