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

Status: RESOLVED FIXED

(Firefox :: Search, task, P3)

Description

[Matthew N. [:MattN]]

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

Comment 1

[Mark Banner (:standard8)]

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.

Comment 2

[Matthew N. [:MattN]]

(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.

Comment 3

[Mark Banner (:standard8)]

Attachment: Bug 1016808 - Use optional chaining in searchSuggestions.sjs. r?#search-reviewers!

Comment 4

[Mark Banner (:standard8)]

Attachment: Bug 1016808 - Use XMLHttpRequest's responseType = json for search suggestions. r?#search-reviewers!

Depends on D183018

Comment 5

[Pulsebot]

Pushed by [e-mail]:
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

Comment 6

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/2522aeb08d46
https://hg.mozilla.org/mozilla-central/rev/eb503815fedd