429718 - custom search engine plugin can't be added on 10.4.11; reported as "unavailable" or "unknown format"

Status: RESOLVED FIXED

(Camino Graveyard :: General, defect)

Description

[Glenn Pillsbury]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en; rv:1.8.1.14) Gecko/20080409 Camino/1.6 (like Firefox/2.0.0.14)
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en; rv:1.8.1.14) Gecko/20080409 Camino/1.6 (like Firefox/2.0.0.14)

Camino 1.6 complains that the search engine plugin is "unavailable" or in "an unknown format" and won't add it to the list of search engines in the browser.  The engine is being added via a html/javascript link on a web page and using www.searchplugins.net api.  This plugin has worked fine for FF and IE7 for several months before the release of Camino 1.6.

Reproducible: Always

Steps to Reproduce:
1. Go to library.pacific.edu
2. Click on link to "Add "Search PacifiCat" to your Firefox or IE 7 browser!" in right side column
3. Watch error message appear.
Actual Results:  
Error message appears:
"Problem adding the search engine.  Camino was unable to add “Unknown Plug-in” to the web search field. The search plug-in could be unavailable or in an unsupported format."

Expected Results:  
The custom plugin should be added to the browser's list of search engines.

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Confirming; I see this on 10.5.2, but it adds just fine on 10.3.9.

Comment 2

[Stuart Morgan]

The definition is at
http://www.searchplugins.net/createos.aspx?number=15127
I don't see anything obviously unusual about it from glancing over it, so we'll probably have to trace through the parsing.

Comment 3

[Sean Murphy]

Apple's NSXMLParser does not like the XML declaration listed:

<?xml version="1.0" encoding="windows-1252"?>

The "windows-1252" encoding is the problem.  Changing it to UTF8 parses without error.

The parsing does still continue after reporting an error on this first line though, and all of the information is actually obtained, just that the return value of -[NSXMLParser parse] indicates there was an error.

In addition to just checking for a successful NSXMLParser parse, I also validate that information was set for both the engine name and url.  As far as fixing this, we could choose to disregard the -[NSXMLParser parse] return value and just rely on that latter check, or come up with a way to talk the parser into liking that character encoding.

Comment 4

[Stuart Morgan]

Is there something in the file that's an invalid character under that encoding? It would be good to know if this is a bug in the definition or a bug in NSXMLParser.

Comment 5

[Stuart Morgan]

(invalid for an XML document, that is)

Comment 6

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(In reply to comment #5)
> (invalid for an XML document, that is)

I'd expect Gecko to barf when displaying the definition file in that case, though that's not my field (I do think I've seen Gecko barf at least once before when this was case).  TextWrangler's pretty good at yelling about invalid characters for the encoding, and it, too, seems happy.

OTOH, a number of online XML validators are throwing errors for the use of the win-1252 declaration in the <?xml?> element (but lots more, including feedvalidator.org, are not):
http://www.stg.brown.edu/service/xmlvalid/
http://www.cogsci.ed.ac.uk/~richard/xml-check.html

As you might expect, it looks like every description from searchplugins.net will fail.

Comment 7

[Stuart Morgan]

The error it's giving is NSXMLParserUnknownEncodingError; that's odd, since NSWindowsCP1252StringEncoding is one of the valid NSStringEncodings. We should report this as a bug to Apple, it looks like.

Comment 8

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

What's the plan here?  Sam's says we're getting Feedback about http://www.searchplugins.net/ plug-ins failing.

It seems like there's no hope for Apple fixing this on 10.4, so we need something there regardless of whether we can convince them to fix it in 10.5.x (and has anyone filed that yet?).

Comment 9

[Stuart Morgan]

I'd be fine with the suggestion in comment 3. As long as all the characters are ASCII, we should be okay, which is better than we are now.

The XML spec says: "Although an XML processor is required to read only entities in the UTF-8 and UTF-16 encodings, it is recognized that other encodings are used around the world, and it may be desired for XML processors to read entities that use them." So this isn't really an Apple bug per se, just an enhancement request.

I've filed that request as rdar://5893203.

Comment 10

[Stuart Morgan]

I also contacted searchplugins.net and suggested that they switch their generator over to UTF-8, but even if they did that it seems unlikely that they would go back and change all the existing ones.

Comment 11

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

It is an Apple bug in that it did work in 10.3.9 (that, or Apple simply ignored the encoding tags there, or something), right?

Comment 12

[Sean Murphy]

Since the approach in comment #3 is reasonable, I'll make the change to solely verify parsed information based on whether we obtained valid engine attributes.

Comment 13

[Stuart Morgan]

I got a reply from searchplugins.net, and they changed the generator to use UTF-8. Even better, it looks like they are dynamically generated on request, rather than stored, so now they all suddenly work in Camino.

We should still make this change though, since there may be other similar definition files out there.

Comment 14

[Sean Murphy]

Attachment: Patch

Ignore NSXMLParser success status, only verify data manually.

Comment 15

[Stuart Morgan]

Comment on attachment 320213 [details] [diff] [review]
Patch

Reference this bug in the comment so it's clear to someone why we are doing that; sr=smorgan with that change.

Comment 16

[Sean Murphy]

Attachment: Patch 1.1

Changed comment to reference this bug.

Comment 17

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Checked in on the trunk and MOZILLA_1_8_BRANCH in advance of 1.6.1.