5

1644911 - Add crash reporter for Fission subframe crashes

 3 years ago
source link: https://bugzilla.mozilla.org/show_bug.cgi?id=1644911
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
neoserver,ios ssh client
Open Bug 1644911 Opened 6 months ago Updated 5 days ago

Add crash reporter for Fission subframe crashes

Categories

(Firefox :: General, enhancement, P3)

Tracking

(ASSIGNED bug which is in the backlog of work awaiting an answer on a request for information)

ASSIGNED

People

(Reporter: cpeterson, Assigned: enndeakin, NeedInfo)

References

Details

attachment.cgi?id=9155788

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]

Previous comment was sent to the wrong bug.

Assignee: nobody → mjaritz
Status: NEW → ASSIGNED
Points: --- → 3
Priority: -- → P3
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]

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

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)
You need to log in before you can comment on or make changes to this bug.

About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK