Closed Bug 1357297 Opened 3 years ago Closed 3 years ago

DecDoc "Report Site Issue" should only be shown for demuxing/metadata issue

Categories

(Core :: Audio/Video: Playback, enhancement)

55 Branch
enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: gerald, Assigned: gerald)

References

Details

Attachments

(2 files)

Following bug 1343442, WebCompat reports have started coming in at a fairly high rate, most of them for decoding errors during playback, which in general we probably won't be able to fix as they can be dependent on the user's hardware and software configurations.

So we should try to reduce the situations in which we show the "Report Site Issue" notification&button, for easier triage and more changes to catch issues we can solve.

Initial idea: Only show notifications for NS_ERROR_DOM_MEDIA_DEMUXER_ERR, NS_ERROR_DOM_MEDIA_METADATA_ERR, maybe a few others?

This white-list should be stored in a pref, to make it easy to test and update as needed.
Comment on attachment 8859370 [details]
Bug 1357297 - Refactor DecDoctor's ReportAnalysis -

https://reviewboard.mozilla.org/r/131406/#review134112

::: dom/media/DecoderDoctorDiagnostics.cpp:314
(Diff revision 1)
> +// Create a webcompat-friendly description of a MediaResult.
> +static nsString
> +MediaResultDescription(const MediaResult& aResult, bool aIsError)
> +{
> +  nsCString name;
> +  GetErrorName(aResult.Code(), static_cast<nsACString&>(name));

The cast is not necessary. Just pass |name|.
Attachment #8859370 - Flags: review?(jwwang) → review+
Comment on attachment 8859371 [details]
Bug 1357297 - Restrict media decode issues that show user notification -

https://reviewboard.mozilla.org/r/131408/#review134114
Attachment #8859371 - Flags: review?(jwwang) → review+
Comment on attachment 8859370 [details]
Bug 1357297 - Refactor DecDoctor's ReportAnalysis -

https://reviewboard.mozilla.org/r/131406/#review134112

> The cast is not necessary. Just pass |name|.

Thank you for the review.

Strange, I'm sure I needed this cast at some point in the past -- or maybe it was elsewhere. But it works without it, great!
Pushed by gsquelart@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d6a68652c1d8
Refactor DecDoctor's ReportAnalysis - r=jwwang
https://hg.mozilla.org/integration/autoland/rev/967c7416140f
Restrict media decode issues that show user notification - r=jwwang
You need to log in before you can comment on or make changes to this bug.