Closed Bug 1644911 Opened 11 months ago Closed 4 months ago

Add crash reporter for Fission subframe crashes

Categories

(Firefox :: General, enhancement, P3)

enhancement
Points:
3

Tracking

()

RESOLVED FIXED
86 Branch
Fission Milestone M6c
Tracking Status
firefox86 --- fixed

People

(Reporter: cpeterson, Assigned: enndeakin)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

Attachments

(3 files, 1 obsolete file)

Steps to reproduce

  1. Enable fission.autostart pref and restart Firefox.
  2. Load https://developer.mozilla.org/en-US/docs/Web/HTML/Element/iframe
  3. Open about:processes
  4. Look for process name "https://openstreetmap.org (webIsolated)" and find its associated Process Id.
  5. Open Activity Manager or Windows Task Manager.
  6. Kill the process with openstreetmap.org's process id.

Expected results

The user should have some way to submit a crash report for the iframe crash.

TBD whether this is the full "Gah. Your tab just crashed." reporter (like about:crashcontent), a door hanger popup, or some other UI.

TBD whether the crashed iframe should be reloaded, show some new "sad iframe" icon, or left blank.

Actual results

The map iframe is replaced with the red "sad tab" icon. See the attached screenshot. The user has no opportunity to submit a crash report for the iframe crash.

Even if the crashed iframe's dimensions are big enough to show the full "Gah. Your tab just crashed." reporter, Firefox just shows the red "sad tab" icon.

We'd like to submit crash reports for Fission iframe crashes. Currently, we just show a "sad tab" icon in the crashed iframe.

Fission Milestone: M6b → ---
Component: Crash Reporting → Firefox Desktop: Project Request
Product: Toolkit → User Experience Design
Summary: Fission iframe crash doesn't open the crash reporter, just shows a "sad tab" icon in the iframe → Need crash reporter UX/UI help for Fission iframe crashes
Whiteboard: [fission:m6b]
Attachment #9160988 - Attachment is obsolete: true

Previous comment was sent to the wrong bug.

Assignee: nobody → mjaritz
Status: NEW → ASSIGNED
Points: --- → 3
Priority: -- → P3

I am collecting Fission Crash Reporter UX requirements in a living doc here: https://docs.google.com/document/d/1rq1C_Bpk3FdBOOxD22OgvliaPQuM3lwbGk1FktWHxXU/edit

Depends on: 1660889
Assignee: mjaritz → enndeakin
Severity: -- → N/A
Component: Firefox Desktop: Project Request → General
Product: User Experience Design → Firefox
Summary: Need crash reporter UX/UI help for Fission iframe crashes → Need crash reporter for Fission iframe crashes
Fission Milestone: --- → M6c
Whiteboard: [fission:m6b]
See Also: → 1677208

This patch adds a notification bar that appears when a subframe crashes. Still to do:

  • the document specifies that the buttons are 'Report to Firefox' and 'Ignore'. What labels should be used? The X button that already exists for notifications would be the same as 'Ignore'. If 'Report to Firefox' is used, then 'Report to Nightly' would appear in nightly builds.
  • the document has a potentially complicated and unclear way to display the notifications when a process crashes in multiple tabs. Need to work out the specifics here.
  • the event 'oop-browser-crashed' that fires only contains a browser context id. Need to add a crash report id so that this report can be submitted.

(In reply to Neil Deakin from comment #7)

  • the document specifies that the buttons are 'Report to Firefox' and 'Ignore'. What labels should be used? The X button that already exists for notifications would be the same as 'Ignore'. If 'Report to Firefox' is used, then 'Report to Nightly' would appear in nightly builds.

I still need to get official wording for the button labels from the UX Content Strategy team (ETA: Early December). In the meantime, the UX team says "Report to Firefox/Nightly" and "Ignore" are adequate placeholder labels for the Nightly channel. So we can land these placeholder labels and then change them before Fission rides the trains.

  • the document has a potentially complicated and unclear way to display the notifications when a process crashes in multiple tabs. Need to work out the specifics here.

The document is a rough work-in-progress, so please feel free to suggest alternatives! :)

See Also: → 1676943

Ed, Tim Spurway tells me you are leading the Proton design for browser notifications.

In this bug, Neil is implementing a new info bar to ask users for permission to submit a crash report for a Fission subframe crash. With Fission, cross-origin subframes (like ads) run in separate content processes, so a subframe on a page can crash without crashing the whole tab.

Will your Proton work impact how we should show this info bar? Neil's current WIP uses gBrowser.getNotificationBox().appendNotification():

https://phabricator.services.mozilla.com/D97346

Flags: needinfo?(edilee)
Summary: Need crash reporter for Fission iframe crashes → Add crash reporter for Fission subframe crashes

Two questions also to add:

  • does the ignore button just close the notification, or does it mean 'don't submit and just delete the crash report'

  • one other thing to handle is when multiple subframes crash, do we get only one crash report or notification? Does the single notification submit information about all of them?

Attachment #9188391 - Attachment description: Bug 1644911, add notification bar when a subframe crashes that allows submitting a crash report → Bug 1644911, add notification bar when a subframe crashes that allows submitting a crash report, r=mconley

This version has a submit button that submits the crash report (or appears in about:crashes as if it did at least), and an ignore button that doesn't submit it. If a subframe crashes in multiple tabs, the notification appears in all of the tabs, but either submitting or ignoring the notification closes the notification in any other tabs which had that crash. This seems to be the simplest way to fit into the existing notification bar mechanism.

If that is ok for now, then this is ready for review.

(In reply to Neil Deakin from comment #10)

does the ignore button just close the notification, or does it mean 'don't submit and just delete the crash report'

This version has a submit button that submits the crash report (or appears in about:crashes as if it did at least), and an ignore button that doesn't submit it.

Sounds good. "Don't submit and just delete the crash report" seems best.

If we just close the notification, then the unsubmitted crash report will be discovered by the unsubmitted crash report scan when Nightly is next started. We don't need to prompt the user about the same crash report twice, especially after they already chose NOT to submit when they had more context about which page crashed.

However, note that the current UI proposal has tentative approval from the UX Design team, the exact wording and buttons have NOT been reviewed by the UX Content team yet. So the wording or buttons might change. Whether you want to delay landing the UI until we have the final wording is up to you.

I'm meeting with someone from the UX Content team later this week. If you'd like to join that meeting, just let me know!

one other thing to handle is when multiple subframes crash, do we get only one crash report or notification? Does the single notification submit information about all of them?

If a subframe crashes in multiple tabs, the notification appears in all of the tabs, but either submitting or ignoring the notification closes the notification in any other tabs which had that crash. This seems to be the simplest way to fit into the existing notification bar mechanism.

That sounds fine. Showing a single notification for all the subframe crashes sounds best. We don't want to bother the user with multiple notifications. And the notification UI doesn't have any different details or context for each subframe crash, so there is no different information to share with the user.

Flags: needinfo?(enndeakin)
Flags: needinfo?(enndeakin)

Neil, here are the final strings approved by UX and Legal:

[icon] Part of this page crashed. To let Firefox know about this issue and get it fixed faster, please submit a report. Learn more [Submit Report] [X]

Good news: looks like your D97346 patch has the correct strings and formatting, except for the new "Learn more" link. It should be a link to SUMO's "Firefox crashes - Troubleshoot, prevent and get help fixing crashes" page:

https://support.mozilla.org/kb/firefox-crashes-troubleshoot-prevent-and-get-help

The "Learn more" anchor text should be localized, but the SUMO URL should NOT be localized because the SUMO site will automatically redirect to a localized page based on the user's HTTP headers or GeoIP region.

btw, [icon] is not finalized yet, but you can land using the red "sad tab" icon until we hear otherwise from UX.

Flags: needinfo?(enndeakin)

Also, does any of this code need to be pref'd off depending on the Fission pref? Or can this code safely ride the trains before Fission does?

Depends on: 1686570

Chris, which of the brand property keynames is expected to be used in the string?
To let <brandname> know about...

The key names are listed in:
Nightly is at: https://searchfox.org/mozilla-central/source/browser/branding/nightly/locales/en-US/brand.ftl
Release is at: https://searchfox.org/mozilla-central/source/browser/branding/official/locales/en-US/brand.ftl

Flags: needinfo?(enndeakin) → needinfo?(cpeterson)

(In reply to Neil Deakin from comment #16)

Chris, which of the brand property keynames is expected to be used in the string?
To let <brandname> know about...

The key names are listed in:
Nightly is at: https://searchfox.org/mozilla-central/source/browser/branding/nightly/locales/en-US/brand.ftl
Release is at: https://searchfox.org/mozilla-central/source/browser/branding/official/locales/en-US/brand.ftl

Let's use -brand-product-name (so we use "Firefox" on all channels). I think showing a channel-specific string like "Nightly" is less informative than "Firefox".

I realize this notification message is using the term "Firefox" is an unusual way: "let Firefox know about this issue". I hope this usage doesn't make translations awkward. :(

Flags: needinfo?(cpeterson) → needinfo?(enndeakin)
Depends on: 1687039
Blocks: 1687855
Pushed by neil@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6a279dc21eb1
add access to the childID from the frame crashed event, r=nika
https://hg.mozilla.org/integration/autoland/rev/a8fca70d9738
add notification bar when a subframe crashes that allows submitting a crash report, r=mconley,fluent-reviewers,flod
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → 86 Branch
Flags: needinfo?(enndeakin)
Flags: needinfo?(edilee)

Neil, I verified the Fission subframe crash reporter is displayed (by loading cnn.com and killing an ad frame's process in about:processes), but when I click the "Submit Report" button, I don't see any new crash ID listed in about:crashes. What is the expected behavior?

Flags: needinfo?(enndeakin)

It works ok for me. I see an unsubmitted id appear once the notification appears and it changes to submitted after clicking "submit report". The about:crashes needs to be reloaded.

Note that in a nightly build, you need to pass --enable-crash-reporter for crash reports to be generated. Perhaps the submit button should be disabled or hidden in this case since it has no effect?

Flags: needinfo?(enndeakin)
Depends on: 1689126
Depends on: 1690916
Depends on: 1691576
No longer depends on: 1691576
You need to log in before you can comment on or make changes to this bug.