349431 - Wikipedia wants to install a search-plugin over and over again

Status: VERIFIED FIXED

(Firefox :: Search, defect)

Description

[Jo Hermans]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b2) Gecko/20060820 BonEcho/2.0b2

A week ago, I noticed that the search-button turned blue when I visited <http://en.wikipedia.org>, indicating that a search-plugin could be installed for this page. So I installed it with the automatic method using https://addons.mozilla.org/data/www/addons.mozilla.org/public/htdocs/search-engines/ (which doesn't work riught now, but that's smtg else).

Today I noticed that the left button it *still* blue, and I still get the offer to install the search-plugin, although it's already there.

On a page like <http://en.wikipedia.org/wiki/Main_Page>, you can find a link to <http://en.wikipedia.org//w/opensearch_desc.php>, with these contents :
<?xml version="1.0"?>
<OpenSearchDescription xmlns="http://a9.com/-/spec/opensearch/1.1/">
<ShortName>Wikipedia</ShortName>
<Description>Wikipedia</Description>
<Image height="16" width="16" type="image/x-icon">http://en.wikipedia.org/favicon.ico</Image>
<Url type="text/html" method="get" template="http://en.wikipedia.org/w/index.php?title=Special:Search&amp;search={searchTerms}"/>
</OpenSearchDescription>

When you install it, you get his wikipedia.xml file in your profile :
<SearchPlugin xmlns="http://www.mozilla.org/2006/browser/search/" xmlns:os="http://a9.com/-/spec/opensearch/1.1/">
<os:ShortName>Wikipedia</os:ShortName>
<os:Description>Wikipedia</os:Description>
<os:Image width="16" height="16">data:image/x-icon;base64,AAABAAEAEBAQAAEABAAoAQAAFgAAACgAAAAQAAAAIAAAAAEABAAAAAAAAAAAAAAAAAAAAAAAEAAAAAAAAAAEAgQAhIOEAMjHyABIR0gA6ejpAGlqaQCpqKkAKCgoAPz9/AAZGBkAmJiYANjZ2ABXWFcAent6ALm6uQA8OjwAiIiIiIiIiIiIiI4oiL6IiIiIgzuIV4iIiIhndo53KIiIiB/WvXoYiIiIfEZfWBSIiIEGi/foqoiIgzuL84i9iIjpGIoMiEHoiMkos3FojmiLlUipYliEWIF+iDe0GoRa7D6GPbjcu1yIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA</os:Image>
<os:Url type="text/html" method="GET" template="http://en.wikipedia.org/w/index.php?title=Special:Search&amp;search={searchTerms}">
</os:Url>
</SearchPlugin>

But that isn't recognized as being the same as the file before.

Comment 1

[Ria Klaassen (not reading all bugmail)]

Attachment: Screenshot of the problem

Comment 2

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

This happens because Wikipedia doesn't include a title in their <link rel="search"> element. Comparing the title of that element to the list of names of installed search plugins is the only way we determine whether or not to show the "has a search" indication, since we haven't downloaded the engine description file when we determine whether to show the indication.

I think Pam had some thoughts on improving this (perhaps by downloading the file and comparing it's contents to installed plugins before showing the indication), but I'm not sure if she'd filed a bug on it already (I can't find any at the moment). Perhaps this could be that bug.

Comment 3

[Pam Greene]

Preloading the engine would be the most robust solution, covering the most cases, but it's also arguably the most heavyweight.  The idea I'd had to balance applicability with effort was to identify engines by the URL of their description files.  That's been part of the patches for both bug 340604 and bug 347400, but neither of them made it in.  It's not a trivial change (nor would preloading be trivial), but it'd certainly be possible to add it.

Comment 4

[Pam Greene]

(In reply to comment #3)
> That's been part of the patches for both bug 340604 and bug 347400

Sorry, I mean bug 339735, not 347400.

Comment 5

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

*** Bug 350223 has been marked as a duplicate of this bug. ***

Comment 6

[Mike Beltzner [:beltzner, not reading bugmail]]

From the dupe ..:

"Nominating for blocking, as I can see this being really annoying, not to
mention the fact that it breaks the meaning of the blue-glow in the search
chrome."

We've previously minused the bugs Pam refers to in comment 4, but based on this effect, I wanted to put the question before drivers again: is this a serious enough issue to block on.

Sadly, I'm not sure how many sites will exist without titles.

Also, fwiw, IE7 simply doesn't offer to add search engines without "title" attributes.

Comment 7

[Jo Hermans]

(In reply to comment #6)
> Also, fwiw, IE7 simply doesn't offer to add search engines without "title"
> attributes.

That seems to be the best solution. 

Comment 8

[Pam Greene]

(In reply to comment #7)
> (In reply to comment #6)
> > Also, fwiw, IE7 simply doesn't offer to add search engines without "title"
> > attributes.
> 
> That seems to be the best solution. 

Unfortunately, it doesn't solve the problem.  The same thing will happen any time the <link> tag on the page presents a title that differs from the one in the OpenSearch description file.

Comment 9

[Mike Connor [:mconnor]]

We should ignore plugins without titles.  It may not completely solve the problem, but it makes us consistent with IE7 and solves a major subset of the problem, so we should do that as an easy road forward to solve the glaring cases in such a way that sites can fix this themselves.

Comment 10

[Pam Greene]

Sites with no titles aren't broken, according to the OpenSearch spec <http://opensearch.a9.com/spec/1.1/description/>; titles aren't required for autodiscovery. It's better to offer a valid engine too often, with a glow that's not terribly intrusive, than to never offer it at all.

Comment 11

[Pam Greene]

Attachment: Refuse engines with no title

In accordance with further discussion with mconnor, this patch ignores <link> tags with missing or empty titles.

Comment 12

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

Comment on attachment 235986 [details] [diff] [review]
Refuse engines with no title

>Index: base/content/browser.js

>+    // Bug 349431: If the engine has no suggested title, ignore it rather
>+    // than trying to find an alternative.
>+    if (!etitle || etitle == "")
>+      return;

!"" is true, so the second check is redundant.

Comment 13

[Pam Greene]

(In reply to comment #12)
> !"" is true, so the second check is redundant.

Huh, I thought I'd encountered a case where it wasn't.  I must have been confused.   Anyway, thanks; I'll take it out before landing.

Comment 14

[Mike Connor [:mconnor]]

Comment on attachment 235986 [details] [diff] [review]
Refuse engines with no title

ok, r=me on this, module gavin's nit.  Though, we never use etitle other than the null-check, so just drop it and use target.title directly.

Comment 15

[Jo Hermans]

I've taken the liberty to notify the maintainer of MediaWiki, hoping that they'll also fix the problem at their end.

http://bugzilla.wikimedia.org/show_bug.cgi?id=2964

Comment 16

[Pam Greene]

Attachment: as checked in on trunk

Comment 17

[Pam Greene]

Attachment: as really checked in on trunk (no tabs)

Comment 18

[Pam Greene]

Comment on attachment 236245 [details] [diff] [review]
as really checked in on trunk (no tabs)

Requesting branch approval. This is a low-risk patch that has been baking on trunk for a day so far.

Comment 19

[Mike Connor [:mconnor]]

Comment on attachment 236245 [details] [diff] [review]
as really checked in on trunk (no tabs)

a=mconnor on behalf of drivers for 1.8 branch checkin

Comment 20

[Pam Greene]

Landed on 1.8.1 branch.

Comment 21

[Jo Hermans]

verified in
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1b2) Gecko/20060907 BonEcho/2.0b2

<http://en.wikipedia.org/> doesn't offer to install the plugin anymore (even if it isn't installed yet). You can easily check by going to another language, like <http://de.wikipedia.org/>.

<http://www.technorati.com/> is recognized correctly - it only offers to install the plugin once, if it doesn't exists yet.

<http://www.theregister.co.uk/> still keeps offering the searchplugin every time, but that's because there's a case-difference in the title-attribute ('El Reg search' in the page, 'El Reg Search' in the plugin). Sigh.

Comment 22

[Murray]

> <http://www.theregister.co.uk/> still keeps offering the 
> searchplugin every time, but that's because there's a 
> case-difference in the title-attribute ('El Reg search' 
> in the page, 'El Reg Search' in the plugin). Sigh.

We've fixed the case difference now.