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

Media playback team bug reporting to webcompat.com #1356

Closed
karlcow opened this issue Feb 23, 2017 · 24 comments
Closed

Media playback team bug reporting to webcompat.com #1356

karlcow opened this issue Feb 23, 2017 · 24 comments

Comments

@karlcow
Copy link
Member

karlcow commented Feb 23, 2017

In summary, I'm in the media playback team, I'm working on a feature where I catch decode errors and I show a notification with a "Report Site Issue" button going to webcompat (similar to what's in the hamburger menu). But I'd like to fill in more of the fields than just 'src'... So would it be possible to 1. do such a thing, and 2. get someone from
your team to help with this?
The information I'd like to add, in addition to the page's src, is:

  • Error code,
  • message (including some technical details,
  • and the C++ function name),
  • and the resource's URL.

by @squelart

@karlcow
Copy link
Member Author

karlcow commented Feb 23, 2017

That's very cool.
the form for sending issue is at:

We probably would benefit to keep a clean structure for these data, so we can parse them correctly and make a nice layout in webcompat.com. Also to avoid weird characters and filtering into markdown.

@squelart Our form currently is loosely structured. We could probably improve that.
Do you have an example of what it could look like for each type of information listed above.

  • URL is a URL (no issue)
  • Error format
  • message (with technical details)
  • C++ function name

Thanks.

@karlcow
Copy link
Member Author

karlcow commented Feb 23, 2017

Hmm is looking at the code and I guess that would be a good opportunity to improve our form handling.

@squelart
Copy link

@karlcow Thank you for opening this issue.
I'm already able to open the webcompat form and add the URL, that was easy, good job there. ;-)

Here are some examples of information I'd like to get into a webcompat form, in addition to the page's URL:

  • Error code, e.g.: NS_ERROR_DOM_MEDIA_DEMUXER_ERR (0x806e000c)
  • Function in which it happened, e.g.: virtual RefPtrMP4Demuxer::InitPromise mozilla::MP4Demuxer::Init()
  • Optionally, some details given by the decoder, e.g.: "Incomplete MP4 metadata"
  • URL of the media resource, e.g.: 'file:///Users/gerald/Documents/mozilla/media.mp4'

A simple solution would be to just give the ability to pre-propulate the "Give more details" box with some arbitrary text, in which I could gather all this extra information.

Bonus: It'd be great if I could pre-select the "Video doesn't play" radio button.

A more difficult solution (I'm guessing) would be to add more fields to match.

@karlcow
Copy link
Member Author

karlcow commented Feb 23, 2017

not necessary more difficult. In the end we handle a python dictionary that we format as a payload for GitHub.

{
    'description': u'1. Navigate to: http://www.example.com/\r\n2. \u2026\r\n\r\nExpected Behavior: FOO\r\n\r\nActual Behavior: BAR\r\n',
    'url': u'http://www.example.com/',
    'os': u'Mac OS X 10.12',
    'metadata': u'<!-- @browser: Firefox 53.0 -->\n<!-- @ua_header: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:53.0) Gecko/20100101 Firefox/53.0 -->\n<!-- @reported_with: web -->\n',
    'problem_type': u"Something else - I'll add details below",
    'browser': u'Firefox 53.0'
}

@miketaylr
Copy link
Member

miketaylr commented Feb 23, 2017

Thanks for filing the bug @karlcow! (I was supposed to last week but uh... forgot. 😢)

We could do this in a couple of ways:

  1. go it all via params
    a. have another GET param like extra_info that passed this stuff in and is just automatically appended to the end of the description field.
    a2. have a GET param for problem_type to preselect that -- video in this case.

  2. postMessage a JS object to the form from Firefox, and react to that
    This is currently how we get the screenshot into the form:
    a. get the screenshot as a blob: http://searchfox.org/mozilla-central/source/browser/extensions/webcompat-reporter/content/WebCompatReporter.jsm#145
    http://searchfox.org/mozilla-central/source/browser/extensions/webcompat-reporter/content/wc-frame.js#11-25
    b. postMessage that to webcompat.com: http://searchfox.org/mozilla-central/source/browser/extensions/webcompat-reporter/content/wc-frame.js#11-25
    c. handle the message from the form: https://github.com/webcompat/webcompat.com/blob/master/webcompat/static/js/lib/bugform.js#L77-L93

Any preference @squelart?

@squelart
Copy link

Hi @miketaylr, thank you for following up.

Selfishly, 1 would be easier for me because all I'm (currently) doing is opening a tab with a hand-built URL.

@karlcow
Copy link
Member Author

karlcow commented Feb 23, 2017

  1. Just send a real JSON payload with all the information needed in an HTTP POST. It seems the simpler :)

@miketaylr
Copy link
Member

1 would be easier for me because all I'm (currently) doing is opening a tab with a hand-built URL.

Cool, good to know.

@squelart
Copy link

If JSON is more future-proof, I could send JSON in my GET too. ;-)

@miketaylr
Copy link
Member

Just send a real JSON payload with all the information needed in an HTTP POST. It seems the simpler :)

One more question @squelart --

Do you think it would be more useful to open the /issues/new form pre-filled with this additional data, and then a users can add more info if they like... (sounds like option 1)

Or would you prefer just sending along the data and an issue is created from that? (sounds like @karlcow's option 3)

I think the answer to that question will help us pick between option 1 and option 3. :)

@squelart
Copy link

@miketaylr
From a privacy angle, I think the user should know what information they are actually giving away publicly, before they submit the form.

And they should be given the opportunity to edit it, just like they can already alter the main URL.

Or alternatively, these special fields (old URL, and new extra info) could be made read-only when provided through GET/POST, because altering them could render the submission useless.

(Disclaimer, if you hadn't noticed yet: I'm not a web-dev!)

@karlcow
Copy link
Member Author

karlcow commented Feb 23, 2017

From a privacy angle, I think the user should know what information they are actually giving away publicly, before they submit the form.

yes. This is the front-end facing part of the story.

And they should be given the opportunity to edit it, just like they can already alter the main URL.

yes.

Then once all the information has been edited there is the way we send it through HTTP to webcompat.com. :) Two separate layers.

Maybe what I'm missing to fully understand the context is what is happening on the firefox side, before the HTTP request. When there is a media problem, what is showed to the users.

@squelart
Copy link

squelart commented Feb 23, 2017

@karlcow
Yes, some more explanatory context might help!

When there is some kind of decode error that prevents starting/continuing playback, we will show a notification bar above the page, saying something like "An error occurred while decoding a media resource.", with a "Report Site Issue" button.
Screenshot of my WIP:
decode-error-notification
Pressing the "Report Site Issue" button should work like the one in the hamburger menu, but in this case I'm able to provide some relevant technical information that I'm hoping we can insert directly into the webcompat form.

For the implementation on my side, the code handling the button press will be based on this one:
http://searchfox.org/mozilla-central/rev/60ae6514e4c559c0c234f0e7aefccb101b8beb2e/browser/base/content/browser-media.js#306
As you can see, it just opens a tab with some URL. Something fancier would be possible, but I'd need help for that!

@karlcow
Copy link
Member Author

karlcow commented Feb 24, 2017

@squelart Thanks. It helps a lot.

so indeed two solutions:

  1. Either handling the form to edit by the user on your side and then you can send it with an HTTP POST on webcompat.com
  2. Or indeed prepopulating the page on webcompat.com with parameters (this can become quickly ugly)

Thinking

@karlcow
Copy link
Member Author

karlcow commented Feb 24, 2017

Food for thought
https://en.wikipedia.org/wiki/Post/Redirect/Get

@miketaylr
Copy link
Member

miketaylr commented Feb 24, 2017

Or indeed prepopulating the page on webcompat.com with parameters (this can become quickly ugly)

So the media team is not blocked forever on an elegant solution (which would probably require UX on the Firefox side), I think we should go this route and file bugs to come up with nicer things in the future, and migrate towards them. I'll file some follow up bugs.

@miketaylr
Copy link
Member

miketaylr commented Feb 24, 2017

@miketaylr
Copy link
Member

@miketaylr
From a privacy angle, I think the user should know what information they are actually giving away publicly, before they submit the form.

I agree with this, FWIW.

@miketaylr
Copy link
Member

Heya @squelart -- just an update here. Back from PTO + manager work week, and #1360 is on my plate for this week.

@squelart
Copy link

Welcome back @miketaylr , the Firefox code is pretty much complete, I need to finish writing front-end tests before sending it for review...
Locally I've already tested details from #1358 and problem_type from #1359 and they worked great. Using #1360 should be trivial for me.

@miketaylr
Copy link
Member

@squelart cool! I think it's safe to plan on there being a label=foo param you can use, and sending it type-media for now. So, label=type-media. I just started sketching my ideas in code, so we'll know a bit more in the next 2 days. 😄

@squelart
Copy link

squelart commented Apr 14, 2017

@miketaylr I probably should have told you in advance, but the media errors notification bar has recently landed in Nightly, so there may be a bit of traffic coming from that now. Please contact me if you see anything bad...
Thank you for your and your team's help and hard work, much appreciated!

@adamopenweb
Copy link
Collaborator

adamopenweb commented Apr 16, 2017

@miketaylr We are seeing issues like this one, which are tagged with:

<!-- @reported_with: media-decode-error -->

as we would expect for issues that contain something like:

Technical Information:
Error Code: NS_ERROR_DOM_MEDIA_FATAL_ERR (0x806e0005)
{{snip}}

in the description.

Then other issues like this one that are reported are not labeled as media:

<!-- @reported_with: web -->

but also contain in the description:

Technical Information:
Error Code: NS_ERROR_DOM_MEDIA_FATAL_ERR (0x806e0005)
{{snip}}

Edit: maybe this comment probably belongs in #1360 instead.

Edit again: I filed a bug for it #1516

@miketaylr
Copy link
Member

This can be closed now, I think!

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

No branches or pull requests

5 participants