Closed Bug 1016808 Opened 10 years ago Closed 11 months ago

Use XMLHttpRequest's responseType = "json" for search suggestions

Categories

(Firefox :: Search, task, P3)

task

Tracking

()

RESOLVED FIXED
117 Branch
Tracking Status
firefox117 --- fixed

People

(Reporter: MattN, Assigned: standard8)

Details

(Whiteboard: [fxsearch])

Attachments

(2 files)

responseType[1] will clean up the code to avoid our our JSON.parse error handling afterwards although we may have to be careful with how it interacts with server backoff code and if we don't currently require the proper mimetype, we may also need overrideMimeType[2]

[1] https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest#responseType
[2] https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/overrideMimeType
Priority: -- → P4
Whiteboard: [fxsearch]
Rank: 46
Severity: minor → N/A
Type: defect → task
Rank: 46
Priority: P4 → P3

Hi Matt, can you clarify a bit more about the concerns here please?

(In reply to Matthew N. [:MattN] from comment #0)

responseType[1] will clean up the code to avoid our our JSON.parse error
handling ...

I'm assuming this is referring to the fact we'll no longer need to do manual parsing here: https://searchfox.org/mozilla-central/rev/a48e21143960b383004afa9ff9411c5cf6d5a958/toolkit/components/search/SearchSuggestionController.jsm#511

...afterwards although we may have to be careful with how it interacts
with server backoff code...

Do you remember the "server backoff code" that we're concerned about here? As far as I know suggestions don't have any backoff code.

and if we don't currently require the proper
mimetype, we may also need overrideMimeType[2]

We don't specify a mimeType currently, so I guess we should probably override it, just in case there's a server that's returning the wrong type somewhere. Alternately the other option would be to test the major ones, and if they work, then we specify the expected response type(s), and skip the override type. Worst case, then I think suggestions wouldn't appear.

Flags: needinfo?(mozilla+bmo)

(In reply to Mark Banner (:standard8) from comment #1)

Hi Matt, can you clarify a bit more about the concerns here please?

(In reply to Matthew N. [:MattN] from comment #0)

responseType[1] will clean up the code to avoid our our JSON.parse error
handling ...

I'm assuming this is referring to the fact we'll no longer need to do manual parsing here: https://searchfox.org/mozilla-central/rev/a48e21143960b383004afa9ff9411c5cf6d5a958/toolkit/components/search/SearchSuggestionController.jsm#511

Correct

...afterwards although we may have to be careful with how it interacts
with server backoff code...

Do you remember the "server backoff code" that we're concerned about here? As far as I know suggestions don't have any backoff code.

It was removed in bug 1007979 (see bug 1007979 comment 22 and https://hg.mozilla.org/mozilla-central/rev/bc27aaabefa89b118b2b0bec16f10cd95147a0f0#l3.65).

and if we don't currently require the proper
mimetype, we may also need overrideMimeType[2]

We don't specify a mimeType currently, so I guess we should probably override it, just in case there's a server that's returning the wrong type somewhere. Alternately the other option would be to test the major ones, and if they work, then we specify the expected response type(s), and skip the override type. Worst case, then I think suggestions wouldn't appear.

Either sounds reasonable to me.

Flags: needinfo?(mozilla+bmo)
Assignee: nobody → standard8
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2522aeb08d46
Use optional chaining in searchSuggestions.sjs. r=search-reviewers,jteow
https://hg.mozilla.org/integration/autoland/rev/eb503815fedd
Use XMLHttpRequest's responseType = json for search suggestions. r=search-reviewers,mcheang
Status: NEW → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → 117 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: