Page MenuHomeSoftware Heritage

To Use proper URL building in origin-search.js
ClosedPublic

Authored by Iamshankhadeep on Feb 21 2020, 4:26 PM.

Details

Summary

Previously we did it by appending fragments of strings and manually escaping data, but now we are using dedicated JavaScript interfaces must be used to build search URLs instead of manually building them.

Related to T2286

Diff Detail

Repository
rDWAPPS Web applications
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Iamshankhadeep retitled this revision from To Use proper URL building in origin-search.js related to T2286 to To Use proper URL building in origin-search.js.Feb 21 2020, 4:30 PM
Iamshankhadeep edited the summary of this revision. (Show Details)
anlambert added a subscriber: anlambert.
anlambert added inline comments.
swh/web/assets/src/bundles/browse/origin-search.js
8

Remove the added white spaces.

I will update our eslint config and add a precommit hook to ensure code style consistency.

100

Use baseSearchUrl.toString() instead

194

Use queryParameters.toString() instead

This revision now requires changes to proceed.Feb 21 2020, 5:57 PM

We can not use queryParameters.toString()
because it returns a string omitting initial '?'. To get the '?' we need to update
window.location.search with queryParameters

swh/web/assets/src/bundles/browse/origin-search.js
194

We can not use

queryParameters.toString()

because it returns a string omitting initial '?'. To get the '?' we need to update

window.location.search
``` with

queryParameters

  • removed spaces and added baseSearchUrl.toString()
anlambert added inline comments.
swh/web/assets/src/bundles/browse/origin-search.js
194

It seems to work without prepending the ? in modern browsers.

Anyway, use the following instead:

window.location.search = `?${queryParameters.toString()}`;

I truly doubt that older browsers support setting an URLSearchParams directly and the documentation is pretty clear that a string should be used.

This revision now requires changes to proceed.Feb 21 2020, 7:36 PM
swh/web/assets/src/bundles/browse/origin-search.js
194

Ohh I get it now.

  • Updated window.location.search with a string
This revision is now accepted and ready to land.Feb 22 2020, 3:06 PM

Updating D2707: To Use proper URL building in origin-search.js