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

Investigate "browser-firefox-mobile-nightly-59.0a1" label on issue #1971

Closed
miketaylr opened this issue Dec 6, 2017 · 15 comments
Closed

Investigate "browser-firefox-mobile-nightly-59.0a1" label on issue #1971

miketaylr opened this issue Dec 6, 2017 · 15 comments

Comments

@miketaylr
Copy link
Member

I don't know why this label would be applied, rather than just browser-fireffox-mobile. We have a few firefox-mobile-nightly labeled issues as well: https://github.com/webcompat/web-bugs/labels/browser-firefox-mobile-nightly

webcompat/web-bugs#14071 is the issue reported by @softvision-oana-arbuzov.

Here's the comment metadata:

<!-- @browser: Firefox Mobile Nightly 59.0a1 (2017-12-04) -->
<!-- @ua_header: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:59.0) Gecko/20100101 Firefox/59.0 -->

Maybe the user added it manually to the browser field? Something for us to normalize in webhook/labeler code.

@karlcow
Copy link
Member

karlcow commented Dec 6, 2017

capture d ecran 2017-12-07 a 00 06 06

@miketaylr
Copy link
Member Author

miketaylr commented Dec 6, 2017

I wonder if fixing #1949 would solve this... if @browser only ever contained the name, presumably version info and dates wouldn't sneak in.

(And we have the original version info in @ua_header if we have doubts).

@karlcow
Copy link
Member

karlcow commented Dec 6, 2017

>>> browser = 'Firefox Mobile Nightly 59.0a1 (2017-12-04)'
>>> import re
>>> re.search(r'([^\d]+?)\s[\d\.]+', browser)
<_sre.SRE_Match object at 0x101a4fd50>
>>> browser = browser.lower()
>>> browser = browser.rsplit(' ', 1)[0]
>>> browser
'firefox mobile nightly 59.0a1'
>>> browser = browser.encode('utf-8')
>>> browser
'firefox mobile nightly 59.0a1'
>>> browser = browser.translate(None, '()')
>>> browser
'firefox mobile nightly 59.0a1'
>>> dash_browser = '-'.join(browser.split())
>>> dash_browser
'firefox-mobile-nightly-59.0a1'
>>> 'browser-{name}'.format(name=dash_browser)
'browser-firefox-mobile-nightly-59.0a1'

The browser.rsplit(' ', 1)[0] doesn't answer this use case.

@karlcow
Copy link
Member

karlcow commented Dec 6, 2017

yes I want to rationalize things and probably #1949 can solve it. I could propose something tomorrow.

@miketaylr
Copy link
Member Author

I completely forgot that we trust user input for the label here... I'm surprised this hasn't been abused yet. >_<

@karlcow
Copy link
Member

karlcow commented Dec 7, 2017

I completely forgot that we trust user input for the label here... I'm surprised this hasn't been abused yet. >_<

I created a new issue #1973 and its associated pull request #1974 before solving this one here.
We problably should fix in #1949 if we think that version-less are ok.

@karlcow
Copy link
Member

karlcow commented Jan 23, 2018

Another one from #15014

capture d ecran 2018-01-23 a 13 53 47

@miketaylr
Copy link
Member Author

I wonder what the best way to fix this is. It seems tricky if we want webcompat-bot to be "smart" and create labels where they don't exist yet (so we don't have to update whitelists or create labels ourselves)

One lazy, not-bullet-proof solution is to reject output that doesn't match [a-z\-]. You could still end up with garbage... but maybe less?

@karlcow
Copy link
Member

karlcow commented Jan 31, 2018

(deleted unrelated comment)

@denschub
Copy link
Member

denschub commented Apr 6, 2018

*slightly pushing this issue*

... moved the brainstorming part into #2372, as an own issue is more appropriate. ...

@karlcow
Copy link
Member

karlcow commented May 9, 2018

Our current UI is
capture d ecran 2018-05-09 a 16 24 46

This is editable by a Human

https://staging.webcompat.com/issues/1499

capture d ecran 2018-05-09 a 16 28 01

no labels have been assigned, because not part of the matching rules.

Let's see
https://staging.webcompat.com/issues/1501
ooopsie. I put in the form:

'Space (Lolipop) 49 and Punk Cat Space' and we end up with
https://github.com/webcompat/webcompat-tests/labels/browser-%27space-lolipop-49-and-punk-cat

capture d ecran 2018-05-09 a 16 35 30

def extract_browser_label(metadata_dict):
"""Return the browser label from metadata_dict."""
browser = metadata_dict.get('browser', None)
# Only proceed if browser looks like "FooBrowser 99.0"
if browser and re.search(r'([^\d]+?)\s[\d\.]+', browser):
browser = browser.lower()
browser = browser.rsplit(' ', 1)[0]
browser = browser.encode('utf-8')
browser = browser.translate(None, '()')
dash_browser = '-'.join(browser.split())
return 'browser-{name}'.format(name=dash_browser)
else:
return None

@karlcow
Copy link
Member

karlcow commented May 9, 2018

Maybe what we can do as a poor man option, but better than nothing.

We currently have two values:

  • The actual browser from where the issue is being reported
  • The browser name typed by the reporter.
  1. If the two are identical, we let the webhooks do its job
  2. If they differ, we tag with a browser-fixme label

The information is already there in the bug report in plain-text.
https://staging.webcompat.com/issues/1501

The fixme will just help to double check which ones need an additional check.

@softvision-oana-arbuzov @softvision-sergiulogigan What do you think?

@karlcow
Copy link
Member

karlcow commented May 31, 2018

@softvision-oana-arbuzov @softvision-sergiulogigan What do you think? :)

karlcow added a commit to karlcow/webcompat.com that referenced this issue May 31, 2018
This intends to not silently fail but instead to mark the issue as something which needs a closer look for the testers/triagers.
karlcow added a commit to karlcow/webcompat.com that referenced this issue May 31, 2018
This doesn't solve everything, but it provides a way to mitigate some of the issues we met. It cut down a lot of possible values and variability. And the fallback being "fixme" if everything else fails, we can count on humans for guessing if possible the browser label.

This doesn't address yet the issue webcompat#2372 we will come to it later.
@karlcow
Copy link
Member

karlcow commented May 31, 2018

I adopted a different strategy. To simplify a bit things.

karlcow added a commit to karlcow/webcompat.com that referenced this issue May 31, 2018
This doesn't solve everything, but it provides a way to mitigate some of the issues we met. It cut down a lot of possible values and variability. And the fallback being "fixme" if everything else fails, we can count on humans for guessing if possible the browser label.

This doesn't address yet the issue webcompat#2372 we will come to it later.
@softvision-sergiulogigan
Copy link
Collaborator

The actual browser from where the issue is being reported
The browser name typed by the reporter.

We do this all the time. We always log issues on webcompat.com using the Desktop (Windows 10), even if the issue itself comes from Mobile, MacOS, Linux etc.

I think that the most relevant information is what the user types in the "Is this information correct?" field.

The fixme will just help to double check which ones need an additional check.

I wonder how much overhead will this create. I'm guessing this will not be an automated process.

karlcow added a commit to karlcow/webcompat.com that referenced this issue Jun 1, 2018
This doesn't solve everything, but it provides a way to mitigate some of the issues we met. It cut down a lot of possible values and variability. And the fallback being "fixme" if everything else fails, we can count on humans for guessing if possible the browser label.

This doesn't address yet the issue webcompat#2372 we will come to it later.
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