Hide the chrome-extension prefix in PDFjs URLs (closes brave/brave-browser#368) #1074
Conversation
|
Note: this patch also fixes the edit URLs for things like |
|
Hi @yrliou, would you be willing to review this patch? Please let me know if I've missed anything process-wise. |
| @@ -32,5 +37,21 @@ base::string16 BraveToolbarModelImpl::GetURLForDisplay() const { | |||
| formatted_text.replace(0, original_scheme_part.length(), | |||
| replacement_scheme_part); | |||
| } | |||
There was a problem hiding this comment.
Previously when I did the security review for using GetURLForDisplay to map brave:// to chrome:// in the URL bar, I assumed the returned URL was not being used for any security checks. However it turns out that assumption was wrong (brave/brave-browser#2631 - sorry you can't see the details yet! can share in Slack).
There was a problem hiding this comment.
actually ignore my above comment, i got brave/brave-browser#213 mixed up with brave/brave-browser#1458
| @@ -18,11 +20,14 @@ const base::string16 original_scheme_part = | |||
| const base::string16 replacement_scheme_part = | |||
| base::ASCIIToUTF16(kInternalUIScheme); | |||
|
|
|||
| const base::string16 pdfjs_url_prefix = | |||
There was a problem hiding this comment.
can we use BraveContentBrowserClient::BrowserURLHandlerCreated to add a HandlerPair for this? That's typically how it would work
Submitter Checklist:
npm test brave_unit_tests && npm test brave_browser_tests) ongit rebase master(if needed).git rebase -ito squash commits (if needed).Test Plan:
Fixes: brave/brave-browser#368
Reviewer Checklist: