1644911 - Add crash reporter for Fission subframe crashes

Status: RESOLVED FIXED

(Firefox :: General, enhancement, P3)

Description

[Chris Peterson [:cpeterson]]

Attachment: iframe_crash_screenshot.jpg

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.

Comment 1

[Chris Peterson [:cpeterson]]

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

Comment 2

[Markus Jaritz [:designakt] (UX)]

Attachment: Bugzilla - bug comment submit fail

'

Comment 3

[Markus Jaritz [:designakt] (UX)]

Previous comment was sent to the wrong bug.

Comment 4

[Chris Peterson [:cpeterson]]

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

Comment 5

[Neha Kochar [:neha]]

Neil, https://docs.google.com/document/d/1rq1C_Bpk3FdBOOxD22OgvliaPQuM3lwbGk1FktWHxXU/edit#heading=h.ptrx0w1p8gq1 has the summary of the desired implementation.

Comment 6

[Neil Deakin]

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

Comment 7

[Neil Deakin]

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.

Comment 8

[Chris Peterson [:cpeterson]]

(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! :)

Comment 9

[Chris Peterson [:cpeterson]]

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

Comment 10

[Neil Deakin]

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?

Comment 11

[Neil Deakin]

Attachment: Bug 1644911, add access to the childID from the frame crashed event, r=nika

Comment 12

[Neil Deakin]

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.

Comment 13

[Chris Peterson [:cpeterson]]

(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.

Comment 14

[Chris Peterson [:cpeterson]]

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.

Comment 15

[Chris Peterson [:cpeterson]]

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?

Comment 16

[Neil Deakin]

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

Comment 17

[Chris Peterson [: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. :(

Comment 18

[Pulsebot]

Pushed by [e-mail]:
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

Comment 19

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/6a279dc21eb1
https://hg.mozilla.org/mozilla-central/rev/a8fca70d9738

Comment 20

[Chris Peterson [:cpeterson]]

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?

Comment 21

[Neil Deakin]

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?