-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
core(url-shim): add intent protocol to NON_NETWORK_PROTOCOLS #6711
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looked around at https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml to see if there's any more that we should add here, and I don't think there are.
(There are other schemes, not none that should trigger network requests in Chromium). Except filesystem:
, but that's deprecated.
So yeah, sg.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -209,6 +209,18 @@ describe('Performance: uses-rel-preload audit', () => { | |||
assert.equal(output.details.items.length, 0); | |||
}); | |||
}); | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Travis is mad about this trailing space. I think this suggestion is accurate to just remove the space but keep the /n
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @midzer !
@@ -209,7 +209,7 @@ URLShim.URL = URL; | |||
URLShim.URLSearchParams = (typeof self !== 'undefined' && self.URLSearchParams) || | |||
require('url').URLSearchParams; | |||
|
|||
URLShim.NON_NETWORK_PROTOCOLS = ['blob', 'data']; | |||
URLShim.NON_NETWORK_PROTOCOLS = ['blob', 'data', 'intent']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here the protocol of intent
request is empty string, you should check the request.parsedURL.scheme instead
And here is a test url: https://mo.m.taobao.com/kgb/20181111.html. The intent
request was sent when in mobile simulator mode
Summary
What kind of change does this PR introduce?
Don't regard an
intent:
request as key requestIs this a bugfix, feature, refactoring, build related change, etc?
Issue been tagged as bug. So bugfix :)
Link any documentation or information that would help understand this change
I'm unsure whether my change/addition goes in the right direction, as I cannot find more detailed information about
intent
protocol/requestsRelated Issues/PRs
closes #6660
Cheers
midzer