Skip to content
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

Redirects additional details button target URL on crashes page. #5798

Merged
merged 3 commits into from
Jun 11, 2020

Conversation

mkarolin
Copy link
Collaborator

@mkarolin mkarolin commented Jun 9, 2020

Fixes brave/brave-browser#10185

Submitter Checklist:

Test Plan:

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

@mkarolin mkarolin added the CI/skip Do not run CI builds (except noplatform) label Jun 9, 2020
@mkarolin mkarolin added this to the 1.12.x - Nightly milestone Jun 9, 2020
@mkarolin mkarolin requested a review from petemill June 9, 2020 21:24
@mkarolin mkarolin requested a review from bridiver as a code owner June 9, 2020 21:24
@mkarolin mkarolin self-assigned this Jun 9, 2020

sendNowButton.remove();
fileBugButton.onclick = () => fileBug(crash.id, os, version);
+ fileBugButton.remove();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should redirect the url to brave-browser issues, but if are going to remove the button I think it can be hidden in css without patching cc @petemill

Copy link
Member

@petemill petemill Jun 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a polymer page, so we need to patch in a crashes_overrides.js or a crashes_overrides.css include to the .html, and remove or hide the button through one of those.

@petemill
Copy link
Member

I agree we can have this pre-fill an issue on github via query params similar to what it looks like chromium is doing
https://help.github.com/en/github/managing-your-work-on-github/about-automation-for-issues-and-pull-requests-with-query-parameters

But if you want to hide it in this PR and create a new issue for that functionality, I think that would be fine too cc @rebron

@rebron
Copy link
Collaborator

rebron commented Jun 10, 2020

Either hide or redirect to github is fine. Thanks for the pointer @petemill

@mkarolin
Copy link
Collaborator Author

Updated PR with redirect instead of hide: @bridiver @petemill

On Crashes page after submitting a crash a button appears that allows
submit a bug with additional details. Instead of directing the user to
bugs.chromium.org we will redirect them to
github.com/brave/brave-browser/issues/new and prefill some of the issue
info.

Fixes brave/brave-browser#10185
@mkarolin mkarolin removed the CI/skip Do not run CI builds (except noplatform) label Jun 11, 2020
@mkarolin mkarolin changed the title Removes additional details button from crashes page. Redirects additional details button target URL on crashes page. Jun 11, 2020
@@ -67,6 +97,9 @@ int OnBeforeURLRequest_CommonStaticRedirectWorkForGURL(
URLPattern::SCHEME_HTTP | URLPattern::SCHEME_HTTPS, kChromeCastPrefix);
static URLPattern clients4_pattern(
URLPattern::SCHEME_HTTP | URLPattern::SCHEME_HTTPS, kClients4Prefix);
static URLPattern bugsChromium_pattern(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bugs_chromium_pattern

@mkarolin mkarolin merged commit b2682c1 into master Jun 11, 2020
@mkarolin mkarolin deleted the maxk-fix-crashes-details branch June 11, 2020 14:03
mkarolin added a commit that referenced this pull request Jun 11, 2020
Redirects additional details button target URL on crashes page.
mkarolin added a commit that referenced this pull request Jun 11, 2020
Redirects additional details button target URL on crashes page.
@kjozwiak
Copy link
Member

Verification PASSED on macOS 10.15.5 x64 using the following build:

Brave | 1.12.18 Chromium: 83.0.4103.97 (Official Build) nightly (64-bit)
-- | --
Revision | 326d148b9655369b86498d9ecca39f63dd2bdd2d-refs/branch-heads/4103@{#657}
OS | macOS Version 10.15.5 (Build 19F101)
  • ensured that both brave://crash and brave://gpucrash are appearing under brave://crashes
  • ensured that clicking on Provide additional details opens GH and not
  • ensured reports are being sent when Help improve Brave's features and performance is enabled via brave://settings/privacy

Created the following crashes and ensured they appeared in BackTrace

  • 7e590000-1c55-8704-0000-000000000000
  • 7d590000-1c55-8704-0000-000000000000
  • 80590000-1c55-8704-0000-000000000000

As per the above, redirecting from bugs.chromium.org -> GH

chromiumLink

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Desktop] brave://crashes has a "Provide additional details" button that links to chromium bug reporting
6 participants