MM-14446: consider subpath when evaluating if url is internal #946
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When clicking on an URL with `target=_blank`, the webview decides if it should launch an external browser or a new window within the Electron application. Update this logic to consider the application's configured subpath so as to treat links outside the subpath but on the same domain as external.
crspeller
approved these changes
Mar 8, 2019
test/specs/utils/util_test.js
Outdated
| @@ -0,0 +1,48 @@ | |||
| // Copyright (c) 2015-2016 Yuya Ochiai | |||
There was a problem hiding this comment.
I admit I don't know the policy here, having just copied the license header from another file. Should I omit @yuya-oc's copyright on this file?
There was a problem hiding this comment.
Well he didn't write it. No idea why it's on the others. I think legally it's fine because the CLA doesn't sign over copyright just an indefinite license.
There was a problem hiding this comment.
Ah, now I know why:
/root/mattermost-desktop/test/specs/utils/util_test.js
3:1 error incorrect header header/header
Fixing this by making the header/header rule apply to a hard-coded list of files, and going forward will only require the mattermost license on new files.
|
Adding HH for a second review before merge (as Yuya is not very available this month) |
|
Given customer urgency we'll take this for a v4.2 dot release |
hmhealey
approved these changes
Mar 15, 2019
hmhealey
added
4: Reviews Complete
|
I just left this as ready to merge since I don't know the process for merging this for a 4.2 dot release |
wget
pushed a commit
that referenced
this pull request
Mar 15, 2019
* MM-14446: consider subpath when evaluating if url is internal When clicking on an URL with `target=_blank`, the webview decides if it should launch an external browser or a new window within the Electron application. Update this logic to consider the application's configured subpath so as to treat links outside the subpath but on the same domain as external. * fix licensing on new file * fix .eslintrc.json indentation * tweak header eslint rules for specific files
wget
pushed a commit
that referenced
this pull request
Mar 24, 2019
* MM-14446: consider subpath when evaluating if url is internal When clicking on an URL with `target=_blank`, the webview decides if it should launch an external browser or a new window within the Electron application. Update this logic to consider the application's configured subpath so as to treat links outside the subpath but on the same domain as external. * fix licensing on new file * fix .eslintrc.json indentation * tweak header eslint rules for specific files
wget
pushed a commit
that referenced
this pull request
Mar 24, 2019
* MM-14446: consider subpath when evaluating if url is internal When clicking on an URL with `target=_blank`, the webview decides if it should launch an external browser or a new window within the Electron application. Update this logic to consider the application's configured subpath so as to treat links outside the subpath but on the same domain as external. * fix licensing on new file * fix .eslintrc.json indentation * tweak header eslint rules for specific files
JtheBAB
pushed a commit
to JtheBAB/desktop
that referenced
this pull request
Jan 17, 2020
…most#946) * MM-14446: consider subpath when evaluating if url is internal When clicking on an URL with `target=_blank`, the webview decides if it should launch an external browser or a new window within the Electron application. Update this logic to consider the application's configured subpath so as to treat links outside the subpath but on the same domain as external. * fix licensing on new file * fix .eslintrc.json indentation * tweak header eslint rules for specific files
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
When clicking on an URL with
target=_blank, the webview decides if it should launch an external browser or a new window within the Electron application. Update this logic to consider the application's configured subpath so as to treat links outside the subpath but on the same domain as external.Issue link
https://mattermost.atlassian.net/browse/MM-14446
Test Cases
Added unit tests for helper function that tests if the target URL is internal. Tested with a subpath configured and without, verifying that links to different domains and different subpaths on the same domain are handled externally in the live build.