fix(ResourceUrlResolver) #1557
fix(ResourceUrlResolver) #1557
Conversation
Change _baseUri for support Chrome App because Uri.base.origin is limited to scheme who start with http or https
|
Thanks for your contribution! In order for us to be able to accept it, we ask you to sign our CLA (contributor's license agreement). CLA is important for us to be able to avoid legal troubles down the road. For individuals (a simple click-through form): http://code.google.com/legal/individual-cla-v1.0.html |
|
Normaly i have already sign your CLA
|
|
@kleak you probably haven't signed the CLA with the email address used for this PR. Could you please re-sign it with the correct email addres ? Regarding the fix, the code has been modified but not the comment. Could you please update it as well (ie why you're not using |
change the comment
|
I have signed the CLA yesterday before PR with my github email address so tell my me if it's ok |
fix the warning in resolveHtml method
|
@chirayuk the restriction on http/https scheme is needed? |
|
I bumped into this problem too when trying to package with phonegap. Looking forward to seeing it integrated into the main branch :). |
|
Assigning to chirayu to investigate for unanticipated consequences... |
| // Ensures that Uri.base is http/https. | ||
| final _baseUri = Uri.base.origin + ('/'); | ||
| // Reconstruct the Uri without the http or https restriction due to Uri.base.origin | ||
| final _baseUri = "${Uri.base.scheme}://${Uri.base.authority}" + ('/'); |
There was a problem hiding this comment.
Check if Uri.base.authority is empty and throw and error if it is. To do that, you might make this assigned the result of a calling a static function. (It will be initialized lazily so it won't throw an error when relative url support is disabled.) Otherwise, looks good.
|
what is the format of the error i have to throw? |
create _getBaseUri static function for create the _baseUri. todo: have to change the error throw
| if (Uri.base.authority.isEmpty) { | ||
| throw "Uri.base.authority is Empty"; | ||
| } else { | ||
| return "${Uri.base.scheme}://${Uri.base.authority}" + ('/'); |
There was a problem hiding this comment.
You can get rid of the manual string concatenation here and put the "/" inside the 1st string like this:
return "${Uri.base.scheme}://${Uri.base.authority}/";
|
Thanks for the fixes. I added two small comments. LGTM otherwise. |
|
merged. Thanks for your contribution! |
#1543
Change _baseUri for support Chrome App