Extensions menu: strip fragment, query and trailing dots from site-settings label [chromium/src : main]
Emilia Paz (Gerrit)
Emilia Paz added 1 comment![]( "Open in Gerrit")
Thanks for spotting this! Instead of sanitizing the URL itself, we can leverage an existent method. `FormatUrlForSecurityDisplay` omitting the scheme should do the trick
Related details
- Bug Bounty Channel
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Bug Bounty Channel (Gerrit)
Bug Bounty Channel voted and added 1 comment![]( "Open in Gerrit")
Votes added by Bug Bounty Channel
| Auto-Submit | +1 |
| Commit-Queue | +1 |
Thanks for spotting this! Instead of sanitizing the URL itself, we can leverage an existent method. `FormatUrlForSecurityDisplay` omitting the scheme should do the trick
- Bug Bounty Channel
- Emilia Paz
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Emilia Paz (Gerrit)
Emilia Paz added 5 comments![]( "Open in Gerrit")
- file:// URLs: strip fragment via GetWithoutRef(), strip query
string via std::string::find('?'), then strip trailing dots via
find_last_not_of('.')update this to last implementation
base::TrimString(current_site, u". ", ¤t_site);Add a comment that explains why this is needed.
} else if (url.SchemeIs(url::kDataScheme)) {
current_site = u"data:";
} else {I think it's ok to show the full data url. Page info shows the data url (see [screenshot](https://drive.google.com/file/d/1BM4HPLF6_MOTetcCAS8M2SCfx4Gr8GLL/view?usp=drive_link))
Changing that would be a different conversation. I think for this CL we can just address the url for file.
extensions::ui_util::GetFormattedHostForDisplay(*web_contents);Could we use `url_formatter::FormatUrlForSecurityDisplay` for all cases? so not differentiating scheme is file?
And, make this change on chrome/browser/extensions/extension_ui_util.cc so we fix this bug for all cases :)
Related details
- Bug Bounty Channel
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Bug Bounty Channel (Gerrit)
Bug Bounty Channel voted and added 4 comments![]( "Open in Gerrit")
Votes added by Bug Bounty Channel
| Auto-Submit | +1 |
| Commit-Queue | +2 |
4 comments
- file:// URLs: strip fragment via GetWithoutRef(), strip query
string via std::string::find('?'), then strip trailing dots via
find_last_not_of('.')update this to last implementation
done
Add a comment that explains why this is needed.
done
} else if (url.SchemeIs(url::kDataScheme)) {
current_site = u"data:";
} else {I think it's ok to show the full data url. Page info shows the data url (see [screenshot](https://drive.google.com/file/d/1BM4HPLF6_MOTetcCAS8M2SCfx4Gr8GLL/view?usp=drive_link))
Changing that would be a different conversation. I think for this CL we can just address the url for file.
done
extensions::ui_util::GetFormattedHostForDisplay(*web_contents);Could we use `url_formatter::FormatUrlForSecurityDisplay` for all cases? so not differentiating scheme is file?
And, make this change on chrome/browser/extensions/extension_ui_util.cc so we fix this bug for all cases :)
- Emilia Paz
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |