Closed Bug 1226903 Opened 5 years ago Closed 5 years ago

[et] Update searchplugin for eki.ee

Categories

(Mozilla Localizations :: et / Estonian, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: sander.lepik, Assigned: flod)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch eki-fix.patch (obsolete) — Splinter Review
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:43.0) Gecko/20100101 Firefox/43.0
Build ID: 20151116155110

Steps to reproduce:

The affected file: browser/searchplugins/eki-ee.xml

EKI has switched to a newer dictionary and we should fix the URL like this:

http://www.eki.ee/dict/qs2006/index.cgi -> http://www.eki.ee/dict/qs/index.cgi

I've attached a patch that fixes the issue.
Component: Untriaged → Search
Assignee: nobody → sander.lepik
Status: UNCONFIRMED → NEW
Component: Search → et / Estonian
Ever confirmed: true
Product: Firefox → Mozilla Localizations
Summary: Search engine link needs to be updated → [et] Update search URL for eki.ee
Version: Trunk → unspecified
Comment on attachment 8690446 [details] [diff] [review]
eki-fix.patch

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

Setting review flag to myself so it won't get lost.
Attachment #8690446 - Flags: review?(francesco.lodolo)
Comment on attachment 8690446 [details] [diff] [review]
eki-fix.patch

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

Thanks Sander, your patch works fine, but since we're touching this plugin let's fix it completely.

If you check the plugin exposed on the page, you'll notice it also has suggestions and uses a completely different icon (also the website does)
http://www.eki.ee/dict/qs/eki_qs.xml

Let me know if you want to create a new patch, or if you prefer me to do it and let you test the resulting searchplugin.

::: browser/searchplugins/eki-ee.xml
@@ +2,5 @@
>     - License, v. 2.0. If a copy of the MPL was not distributed with this
>     - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
>  
>  <SearchPlugin xmlns="http://www.mozilla.org/2006/browser/search/">
>  <ShortName>Eki</ShortName>

When you visit http://www.eki.ee/dict/qs/index.cgi you'll notice that the search bar asks you to add a new searchplugin, even if you already have one for this URL.

That's because the URL exposes an OpenSearch plugin, with name 'Õigekeelsussõnaraamat'.
We should use the same name, unless it's not useful for Firefox users.

@@ +6,5 @@
>  <ShortName>Eki</ShortName>
>  <Description>Eki dictionary</Description>
>  <InputEncoding>UTF-8</InputEncoding>
>  <Image width="16" height="16">data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8%2F9hAAAACXBIWXMAAAsTAAALEwEAmpwYAAABpUlEQVQ4EaWTu0sDQRDGZ3OXxFj4QohooWDACCIWFmJhIdgk%2F6i9JAgiWtiIhdhZJEYxPoixSCwMarL%2B5nJ7Oa%2BMA3MzszPfN7OPM9WCWPmH%2BGAzMbzBT6Hz6Le18m6MaI3GhlYtvjP4llybnKfFDbSOdhjlDXtcqkkTm6Ggiz3TGH0EfE88CVjIfeA3lH0JVdmhvRIeBpEVD0AOP8iz1Rp%2BByJtKMQtzIoCnDRhvqZguboqE24R26X4AjsgVyTnMGmt0QmcZMt16VHssdCPHe06vi3VxVcwduAAah2b%2Bj%2BAT%2BmiYCUZipWUNVKoFESSYC2IEzwQ70NyQmFviOZrpM%2FZHJU5gcpo%2FCgdEdBZr%2FAJe5A4A63ZgPiKLf4ZX1kiAgryxFu6qF0DO%2Fy4abapSSeniAiozdK9HQLjh%2FvM2mK4fp6cIk6wQAe931eIPkOAGo%2F4BfvF%2B9klvxnmdMujLeBfogrMcwZFtuHIZ1nj7cpegBC50RgJptIi%2FRecTnGFOW7hlq53rOcAzmGF0bWBvky3vWn8jKmuBe8efzzxeeHuXxiL4ReHOoB%2FFscq0QAAAABJRU5ErkJggg%3D%3D</Image>
> +<Url type="text/html" method="GET" template="http://www.eki.ee/dict/qs/index.cgi" resultdomain="eki.ee">

Since search URL and searchform use the same URL, we can add a rel="searchform" here, and drop the <SearchForm> line completely.

See an example here
http://hg.mozilla.org/mozilla-central/diff/9ca3d3875c7d/browser/locales/en-US/searchplugins/twitter.xml
Attachment #8690446 - Flags: review?(francesco.lodolo)
Flags: needinfo?(sander.lepik)
(In reply to Francesco Lodolo [:flod] from comment #2)
> Comment on attachment 8690446 [details] [diff] [review]
> eki-fix.patch
> 
> Review of attachment 8690446 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks Sander, your patch works fine, but since we're touching this plugin
> let's fix it completely.
> 
> If you check the plugin exposed on the page, you'll notice it also has
> suggestions and uses a completely different icon (also the website does)
> http://www.eki.ee/dict/qs/eki_qs.xml
> 
> Let me know if you want to create a new patch, or if you prefer me to do it
> and let you test the resulting searchplugin.
I prefer you to do it, as you know better what exactly needs to be done :)

> ::: browser/searchplugins/eki-ee.xml
> @@ +2,5 @@
> >     - License, v. 2.0. If a copy of the MPL was not distributed with this
> >     - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> >  
> >  <SearchPlugin xmlns="http://www.mozilla.org/2006/browser/search/">
> >  <ShortName>Eki</ShortName>
> 
> When you visit http://www.eki.ee/dict/qs/index.cgi you'll notice that the
> search bar asks you to add a new searchplugin, even if you already have one
> for this URL.
> 
> That's because the URL exposes an OpenSearch plugin, with name
> 'Õigekeelsussõnaraamat'.
> We should use the same name, unless it's not useful for Firefox users.
Yes, 'Õigekeelsussõnaraamat' is better choice for the name.

> @@ +6,5 @@
> >  <ShortName>Eki</ShortName>
> >  <Description>Eki dictionary</Description>
> >  <InputEncoding>UTF-8</InputEncoding>
> >  <Image width="16" height="16">data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQCAYAAAAf8%2F9hAAAACXBIWXMAAAsTAAALEwEAmpwYAAABpUlEQVQ4EaWTu0sDQRDGZ3OXxFj4QohooWDACCIWFmJhIdgk%2F6i9JAgiWtiIhdhZJEYxPoixSCwMarL%2B5nJ7Oa%2BMA3MzszPfN7OPM9WCWPmH%2BGAzMbzBT6Hz6Le18m6MaI3GhlYtvjP4llybnKfFDbSOdhjlDXtcqkkTm6Ggiz3TGH0EfE88CVjIfeA3lH0JVdmhvRIeBpEVD0AOP8iz1Rp%2BByJtKMQtzIoCnDRhvqZguboqE24R26X4AjsgVyTnMGmt0QmcZMt16VHssdCPHe06vi3VxVcwduAAah2b%2Bj%2BAT%2BmiYCUZipWUNVKoFESSYC2IEzwQ70NyQmFviOZrpM%2FZHJU5gcpo%2FCgdEdBZr%2FAJe5A4A63ZgPiKLf4ZX1kiAgryxFu6qF0DO%2Fy4abapSSeniAiozdK9HQLjh%2FvM2mK4fp6cIk6wQAe931eIPkOAGo%2F4BfvF%2B9klvxnmdMujLeBfogrMcwZFtuHIZ1nj7cpegBC50RgJptIi%2FRecTnGFOW7hlq53rOcAzmGF0bWBvky3vWn8jKmuBe8efzzxeeHuXxiL4ReHOoB%2FFscq0QAAAABJRU5ErkJggg%3D%3D</Image>
> > +<Url type="text/html" method="GET" template="http://www.eki.ee/dict/qs/index.cgi" resultdomain="eki.ee">
> 
> Since search URL and searchform use the same URL, we can add a
> rel="searchform" here, and drop the <SearchForm> line completely.

OK :) And thanks for your help!
Flags: needinfo?(sander.lepik)
Sounds good. I'll have a plugin to test for you tomorrow.
Assignee: sander.lepik → francesco.lodolo
Summary: [et] Update search URL for eki.ee → [et] Update searchplugin for eki.ee
Attached patch bug1226903.patchSplinter Review
Here's a patch with the updates.

* ShortName and Description are the same of the official searchplugin
* Updated the icon with 16px+32px
* Added search suggestions, updated search URLs. Added the same parameters used on the website.

For testing you can go here and install the searchplugin
https://l10n.mozilla-community.org/~flod/testsp/

* SearchForm is triggered when you click on the icon without any search term
* To test suggestions you need to temporary set the searchplugin as default
Attachment #8690446 - Attachment is obsolete: true
Attachment #8690675 - Flags: review?(sander.lepik)
(In reply to Francesco Lodolo [:flod] from comment #5)
> Created attachment 8690675 [details] [diff] [review]
> bug1226903.patch
> 
> Here's a patch with the updates.
> 
> * ShortName and Description are the same of the official searchplugin
Tested, OK.

> * Updated the icon with 16px+32px
Tested, OK.

> * Added search suggestions, updated search URLs. Added the same parameters
> used on the website.
Tested, suggestions work as expected, didn't see any issues, so also OK.

Thank you for the great improvement! :)
Attachment #8690675 - Flags: review?(sander.lepik) → review+
Comment on attachment 8690675 [details] [diff] [review]
bug1226903.patch

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

Thanks Sander, flaggin as reviewed
Attachment #8690675 - Flags: review?(sander.lepik) → review+
Looks like we caught Bugzilla in the middle of a huge performance issue and clashed :-\

Landings:
https://hg.mozilla.org/releases/l10n/mozilla-beta/et/rev/2682678ed2ea
https://hg.mozilla.org/releases/l10n/mozilla-aurora/et/rev/2682678ed2ea
https://hg.mozilla.org/l10n-central/et/rev/2682678ed2ea

You will need to ask a new sign-off on Beta to include this in Firefox 43.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.