Closed Bug 335691 Opened 18 years ago Closed 18 years ago

Need better support for OpenSearch parameters

Categories

(Firefox :: Search, defect, P1)

Product:
Firefox
Component:
Search
Version:
2.0 Branch
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 2 beta1

People

(Reporter: Gavin, Assigned: Gavin)

References

Details

(Keywords: fixed1.8.1, Whiteboard: 181b1+)

Attachments

(1 file, 4 obsolete files)

as checked in
17.78 KB, patch
beltzner
: approval1.8.1+
Details | Diff | Splinter Review 
See http://opensearch.a9.com/spec/1.1/querysyntax/#parameter-syntax and http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/components/search/nsSearchService.js&rev=1.17#109
This currently prevents the search service from loading a lot of the commonly distributed OpenSearch description files.
Assignee: nobody → gavin.sharp
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → Firefox 2 beta1
Attached patch step #1 (obsolete) — — Splinter Review 
Fix global constant naming to be consistently ALL_CAPS, add some namespaces to support common OpenSearch plugins, and make "method" optional, default to "GET" (per http://opensearch.a9.com/spec/1.1/querysyntax/#urltag).
Attachment #220124 - Flags: review?(mconnor)
Whiteboard: [patch-r?]
Comment on attachment 220124 [details] [diff] [review]
step #1

grr, this was bitrotted by bug 335691.
Attachment #220124 - Attachment is obsolete: true
Attachment #220124 - Flags: review?(mconnor)
Whiteboard: [patch-r?]
Fixes global constants to be consistent, and a few other minor changes.
Attachment #221059 - Flags: review?(pamg.bugs)
Comment on attachment 221059 [details] [diff] [review]
fix nits (checked in, branch and trunk)

Looks good.  Sorry for trampling the earlier patch.
Attachment #221059 - Flags: review?(pamg.bugs) → review+
Whiteboard: [checkin needed]
Depends on: 337625
mozilla/browser/components/search/nsSearchService.js 	1.23
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed] → [checkin needed (1.8 branch)]
More to do here...
Status: RESOLVED → REOPENED
Priority: P2 → P1
Resolution: FIXED → ---
Status: REOPENED → ASSIGNED
mozilla/browser/components/search/nsSearchService.js 	1.1.2.19
Whiteboard: [checkin needed (1.8 branch)]
Attachment #221059 - Attachment description: fix nits → fix nits (checked in, branch and trunk)
Flags: blocking-firefox2?
Whiteboard: [swag: 1d]
Flags: blocking-firefox2? → blocking-firefox2+
Attached patch patch (obsolete) — — Splinter Review 
Pam, do you mind taking a look at this? I implemented this based on the docs at http://opensearch.a9.com/spec/1.1/querysyntax/#parameter-syntax .
Attachment #221059 - Attachment is obsolete: true
Attachment #226100 - Flags: superreview?(mconnor)
Attachment #226100 - Flags: review?(pamg.bugs)
Attachment #226100 - Flags: approval-branch-1.8.1?(mconnor)
Attachment #226100 - Flags: approval-branch-1.8.1?(mconnor)
Comment on attachment 226100 [details] [diff] [review]
patch

>+const OS_PARAM_IENC         = /\{inputEncoding\??\}/g;
>+const OS_PARAM_LANG         = /\{language\??\}/g;
>+const OS_PARAM_OENC         = /\{outputEncoding\??\}/g;

>+const OS_PARAM_SINDEX       = /\{startIndex\??\}/g;
>+const OS_PARAM_SPAGE        = /\{startPage\??\}/g;

Looks good.  My only nits are with the constant names.  "IENC" and "OENC" could be clearer, and even "SINDEX" and "SPAGE" are a little obscure.  I'd rather type a little more now (perhaps OS_PARAM_OUTENC, if not OS_PARAM_OUTPUT_ENCODING) and save someone else a few minutes of confusion.
Attachment #226100 - Flags: review?(pamg.bugs) → review+
Attached patch with Pam's comments addressed (obsolete) — — Splinter Review 

        Attachment #226100 -
        Attachment is obsolete: true

        Attachment #226427 -
        Flags: superreview?(mconnor)

        Attachment #226100 -
        Flags: superreview?(mconnor)
Whiteboard: [swag: 1d] → [patch-r?]

    
  
Whiteboard: [patch-r?] → [patch-r?] 181b1+

        Attachment #226427 -
        Flags: superreview?(mconnor) → superreview+

    
    

    
  

      
      
      
      

        Attached patch
          
          
        as checked in
        — 
          — Splinter Review
      

    


  
mozilla/browser/components/search/nsSearchService.js 	1.44

        Attachment #226427 -
        Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Closed: 18 years ago → 18 years ago
Resolution: --- → FIXED
Whiteboard: [patch-r?] 181b1+ → 181b1+
Whiteboard: 181b1+ → [need-a] 181b1+

    
    

    
  
Comment on attachment 227411 [details] [diff] [review]
as checked in

No known regressions (though it's only been in one nightly), and I've tested this pretty well. This is a critical piece of Firefox 2's OpenSearch support, and also blocks bug 335460.

        Attachment #227411 -
        Flags: approval1.8.1?

    
    

    
  
Comment on attachment 227411 [details] [diff] [review]
as checked in

You are cleared to land on runway 181, taxi left, contract apron control on 171.9, g'day.

        Attachment #227411 -
        Flags: approval1.8.1? → approval1.8.1+

    
    

    
  
mozilla/browser/components/search/nsSearchService.js 	1.1.2.39
Keywords: fixed1.8.1
Whiteboard: [need-a] 181b1+ → 181b1+

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: