-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Initial Implementation #1
Comments
As for the proxy, there's this thing: https://github.com/digitalbazaar/bedrock-http-proxy/tree/initial/lib Which afaik is not being used. So it can be made to do whatever. Including using some popular express middleware for the proxy bit. |
I don't think that proxy code is sufficient -- we're doing quite a bit more with the current manifest.json proxy here: https://github.com/digitalbazaar/authorization.io/blob/master/lib/http.js The above is also intentionally more restrained in what it can do. So we either need to add features to |
@dlongley Elaboration on questions from office hours. Currently the function that will be called is
First of all I want to make sure that this is the intention of a function that would be called and if so I have further questions.
|
Added potential options above for dark/light mode. |
We would just use
Fallback icons are needed when either the manifest does not exist or the manifest does not list icons. Both scenarios are possible.
We don't need to do that here (another layer handles default icons) -- the only fallback we use is the
We don't need to worry about this here either, I don't think. We'll let another layer handle it. But, the answer to the questions here is that we'd just use the only information available (the origin) to provide defaults. So, the default name of the application would be the domain (
No, we don't need to do this here in the first iteration. |
@dlongley So in a case where the manifest does not have icons, we would fallback to a |
Btw, in the browser, it's not possible for Javascript code to detect that a CORS error has occurred (the browser basically suppresses it silently). We may need to do some sort of timeout mechanism to catch it. |
I didn't mean for my comment to be that prescriptive. If the attempt to get the manifest.json via the client fails, we should try should start moving to fallback options. I do believe, however, that we may be able to differentiate between a "Network Error" and a 404. If so, the manifest fetching could work like this:
If we can't make such a distinction then we'd just do this:
|
Yes.
Yes, that's one option (though perhaps |
I think a goal for this library should be to provide functions that do one thing well: get the manifest directly (over CORS), get the manifest via proxy, find the best icon given some params, etc. And then also provide functions that will combine those functions to give a resilient result that supports defaults when all else fails (if those defaults are provided by the user). So, someone using this library can make the individual calls directly if they want to -- or they can call the helper(s) that will do the above "fallback" steps to give you something to work with that may use your defaults if necessary. |
Closing per 1.0 release. We can make whatever changes are needed as required -- without keeping this perma issue open. |
This module should:
/manifest.json
file from a domain. When called from a browser, this may potentially fail because of CORS. If a CORS error occurs, then this function should fetch the file via a proxy service if one is given in the options.Another module may be needed to provide an express middleware for proxying manifest.json requests; it should not be this module as that code isn't needed by clients.
The text was updated successfully, but these errors were encountered: