413907 - OpenSearch parser doesn't understand XML namespaces

Status: RESOLVED FIXED

(Camino Graveyard :: General, defect)

Description

[Stuart Morgan]

The way the OpenSearch XML parser looks for specific elements doesn't account for the possibility of XML namespaces. I've encountered a plugin that looks like this (sorry, no link available, but making a testcase will be easy):

<Search Plugin xmlns:os="http://a9.com/-/spec/opensearch/1.1"...>
<os:ShortName>...
<os:Description>...
etc.

Because of the namespace prefix, the parser is rejecting the elements since they don't match the built-in list.

Given the very limited domain of things we need to parse, I bet we could get away with just throwing away <anything>: whenever we look at an element to decide what to do with it.

Comment 1

[Sean Murphy]

(In reply to comment #0)

> Given the very limited domain of things we need to parse, I bet we could get
> away with just throwing away <anything>: whenever we look at an element to
> decide what to do with it.
 
Sound like a good enough plan to me; doesn't hurt attempting to work with a wide range of possible input.

During the initial design I was wary of introducing anything related to namespaces, as NSXMLParser pre-10.4 supposedly does not support or handle them very well.  I think the documentation is mainly referring to some of the specific delegate methods and arguments explicitly about namespaces, however, and the actual problem we're dealing with doesn't necessarily require using any of those features.

I made up a quick test case and debugged it, and it seems that the didStartElement... delegate method will just supply the namespace prefix along with the element name, as "os:ShortName", not trying to do anything special as far as breaking up the namespace from the element.  Since that's the case, taking your route should be possible.  I'll take a look at fixing it.

Also, nice catch noticing this problem; I mistakingly never managed to run into and test a plug-in using a namespace prefix.

Comment 2

[Sean Murphy]

Attachment: Branch Patch

I decided to let XMLSearchPlugin parser handle the namespaces issue.  The only problem with ignoring namespaces at this level is that if we ever have any parser subclasses that need to deal with them, it would be the wrong approach.  If you think this will cause future trouble, I can move the behavior down into OpenSearchParser...

Comment 3

[Sean Murphy]

Attachment: Test Case

Contains a test web page + OS definition.

Comment 4

[Stuart Morgan]

For trunk this should probably just use +[NSXMLNode localNameForName:]. I'll review this version for the branch when I get a chance though.

Comment 5

[Stuart Morgan]

Comment on attachment 302829 [details] [diff] [review]
Branch Patch

>+- (NSString *)simplifiedNameForElement:(NSString *)fullElementName;

For consistency with the 10.4+ method that it's re-imlementing, make this a class method and call it localNameForName:.

r=me for branch with that change (and r=me for a trunk version that doesn't have the new method, and uses NSXMLNode localNameForName: in the places this patch uses your new method)

Comment 6

[Sean Murphy]

Attachment: Trunk Patch

Comment 7

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 302829 [details] [diff] [review]
Branch Patch

sr=pink

Comment 8

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 305414 [details] [diff] [review]
Trunk Patch

sr=pink

Comment 9

[Stuart Morgan]

Landed on trunk and MOZILLA_1_8_BRANCH.