904713 - Mobile Web Compatibility Custom Bugzilla form

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: Extensions, defect)

Description

[Karl Dubost💡 :karlcow]

The goal is to have one very simple form for Mobile Web Compatibility bugs for two products:

* Firefox OS
* Firefox for Android

There might be details to discuss

# My comments/instructions
(inline placeholder)
<actions>
::new label
+field  (this means that if possible the field could be 
         broken down in a multiple input fields)

Everything which is not mentioned below should be hidden.
In the things which are mentioned, 
* some of them are hidden but selected.
* some have specific instructions.

If the labels can be changed that would be cool.


Product: Tech Evangelism      # selected and hidden
Component: Mobile             # selected and hidden
OS: <Firefox OS or Android>   # Give the choice of Gonk (Firefox OS) 
                                or Android but not others if possible
Status: unconfirmed           # selected and hidden
Assignee: [e-mail]  # selected and hidden
Summary:                      # mandatory
::Problem summary
 (Give one short sentence 
  explaining the problem)
Description:                  # mandatory - multiline
:: Steps to reproduce the problem?
  (1. 
   2.
   3.
   …)
+Description:                  # mandatory - one line
:: Expected result
  (What were you expecting?)
+Description:                  # mandatory - one line 
:: Actual result
  (What did you get?)
+Description:                  # hidden but collecting information 
                                 about the device. Hmm that would work 
                                 only if people are filling from the 
                                 mobile device.
URL:                           # mandatory - one line

Comment 1

[Karl Dubost💡 :karlcow]

Attachment: webform-mockup.png

This is a mockup for this bug. It might require polishing.

Comment 2

[Gervase Markham [:gerv]]

Attachment: Screenshot

Here's a screenshot of the patch. It has some limitations, as documented below, but I think it's adequate for now.

* The layout and formatting matches other, similar bug filing forms.

* The "if possible, file bugs using your device" message at the top only appears if the user's browser's User Agent does not contain the string "Mobile". That seemed like a reasonable heuristic, given that if we get it wrong it's fairly harmless.

* I added a "Software Version" field because often the user will not be using their device. However, it's currently not auto-filled if they are, and neither is the product automatically selected. I also added an optional "Device Information" field for the same reason.

* The Steps To Reproduce "1. 2. 3." text is actually in the field, not auto-vanishing placeholder text, because we want people to use it as a template. (This means that the "mandatory" nature of this field is not checked, as it already contains text.)

* The Initial Description of the filed bug looks something like this (some data which has its own dedicated field is also reproduced here so the comment can be read standalone):

<snip>
Site: http://www.mozilla.org/
mozilla.org header is corrupted

:: Steps To Reproduce

1. Visit www.mozilla.org
2. See header is misaligned
3.
...

:: Expected Result

Aligned header

:: Actual Result

Misaligned header

:: Additional Information

Software Version: 1.1
Device Information: Geeksphone Peak
Reporter's User Agent: Mozilla/5.0 (X11; Linux i686; rv:26.0) Gecko/20100101 Firefox/26.0
</snip>

Gerv

Comment 3

[Gervase Markham [:gerv]]

Attachment: Patch v.1

Here's the patch.

It's based a little bit on form.ipp, but I genericised the code somewhat so it's easier to base other forms on this one and simply change out the relevant field names and data.

Gerv

Comment 4

[Karl Dubost💡 :karlcow]

Gervase,

Fantastic! Some comments.

(In reply to Gervase Markham [:gerv] from comment #2)
> * The layout and formatting matches other, similar bug filing forms.

Yes no issue and it depends on your stylesheet

> * I added a "Software Version" field because often the user will not be
> using their device. However, it's currently not auto-filled if they are, and
> neither is the product automatically selected. I also added an optional
> "Device Information" field for the same reason.

Ah neat!
Product seems dry, but I do not have a better name for now. So let's keep it until we get a better suggestion. I wonder if people without technical knowledge will know the difference in between Firefox OS and Firefox for Android. This might be an issue in the future. #ToThink

> * The Steps To Reproduce "1. 2. 3." text is actually in the field, not
> auto-vanishing placeholder text, because we want people to use it as a
> template. (This means that the "mandatory" nature of this field is not
> checked, as it already contains text.)

Yes agreed.


Thanks again!

Comment 5

[Karl Dubost💡 :karlcow]

Comment on attachment 790134 [details]
Screenshot

See the comments. Good to go for me.

Comment 6

[Karl Dubost💡 :karlcow]

Important thing: Is it possible for users without account to bugzilla to fill a bug?
If we want to make it possible for anyone to enter a bug we should give this possibility.

Comment 7

[:glob ✱]

(In reply to Karl Dubost from comment #6)
> Important thing: Is it possible for users without account to bugzilla to
> fill a bug?

no, that isn't possible.

Comment 8

[Anthony Ricaud (:rik)]

Karl asked for my comments on this:
- For metrics purpose, it might be useful to set a keyword/whiteboard/flag for bugs entered through this form.
- Even if the form is simplified, the wording is not very engaging. Some suggestions:
  * "Steps to reproduce" -> "Describe what you were doing"
  * "Actual result" -> "What happened?"
  * "Expected result" -> "What were you expecting instead?"
  And I'd place "What happened?" before "Describe what you were doing".

Comment 9

[:glob ✱]

Comment on attachment 790135 [details] [diff] [review]
Patch v.1

Review of attachment 790135 [details] [diff] [review]:
-----------------------------------------------------------------

this mostly looks good.
don't forget to test your code, and run bugzilla's tests.

#   Failed test '(en/default) extensions/BMO/template/en/default/bug/create/create-mobile-compat.html.tmpl has unfiltered directives:
#   81: title
#   92: cgi.user_agent()
# --ERROR'
#   at t/008filter.t line 130.

#   Failed test 'extensions/BMO/template/en/default/bug/create/create-mobile-compat.html.tmpl contains invalid bare words (e.g. 'bug') --WARNING'
#   at t/009bugwords.t line 90.

::: .htaccess
@@ +61,5 @@
>  RewriteRule ^form[\.:](swag|gear)$ enter_bug.cgi?product=mozilla.org&format=swag
>  RewriteRule ^form[\.:](b2g|fxos)[\.:](partner|dogfood) enter_bug.cgi?product=Boot2Gecko&format=fxos-$2 [QSA]
>  RewriteRule ^form[\.:]ipp$ enter_bug.cgi?product=Internet+Public+Policy&format=ipp
>  RewriteRule ^form[\.:]creative$ enter_bug.cgi?product=Marketing&format=creative
> +RewriteRule ^form[\.:]mobile[\.:]compat$ enter_bug.cgi?product=Tech%20Evangelism&format=mobile-compat

%20 won't work here, use + for spaces.

::: extensions/BMO/template/en/default/bug/create/create-mobile-compat.html.tmpl
@@ +5,5 @@
> +  # This Source Code Form is "Incompatible With Secondary Licenses", as
> +  # defined by the Mozilla Public License, v. 2.0.
> +  #%]
> +
> +[% PROCESS global/variables.none.tmpl %]

this line isn't required

@@ +59,5 @@
> +  if (alert_text != '') {
> +    alert(alert_text);
> +    return false;
> +  }
> +  

remove trailing whitespace

@@ +71,5 @@
> +   title = title
> +   style = inline_style
> +   javascript = inline_javascript
> +   javascript_urls = [ 'extensions/BMO/web/js/form_validate.js',
> +                       'js/field.js', 'js/util.js', 'js/bug.js' ] XXXcheck

looks like you left a reminder to check something there.
and, yes, please check that you need all of those includes from js/ (i suspect you don't need any).

@@ +89,5 @@
> +  <input type="hidden" name="bug_status"   value="UNCONFIRMED">
> +  <input type="hidden" name="rep_platform" value="Other">
> +  <input type="hidden" name="bug_severity" value="normal">
> +  <input type="hidden" name="user_agent"   value="[% cgi.user_agent() %]">
> +  

remove trailing whitespace

@@ +95,5 @@
> +
> +<table id="bug_form">
> +
> +[% IF NOT cgi.user_agent("Mobile") %]
> +<p>If possible, it's best to file bugs using your device's browser. Visit and bookmark <a href="http://mzl.la/16KuCao">http://mzl.la/16KuCao</a>.</p>

this needs to be indented, and moved before the table is opened - it's invalid to have <p> directly within <table>.

the url which http://mzl.la/16KuCao points to isn't valid - form.web-compat isn't valid even with this patch.

i would prefer to see the full url here, rather than hiding it behind a shortener:
<a href="https://bugzilla.mozilla.org/form.mobile.compat">bugzilla.mozilla.org/form.mobile.compat</a>
while longer, it would be easier to enter into a mobile device when compared to the mix of random digits and letters from mzl.la.

@@ +190,5 @@
> +
> +[ <span class="required_star">*</span> <span class="required_explanation">Required Field</span> ]
> +
> +<div id="standard_link">
> +  <a href="enter_bug.cgi?format=__standard__&product=[% product.name FILTER uri %]">

s/&/&amp;/

@@ +192,5 @@
> +
> +<div id="standard_link">
> +  <a href="enter_bug.cgi?format=__standard__&product=[% product.name FILTER uri %]">
> +    <img src="extensions/BMO/web/images/advanced.png" width="16" height="16" border="0"></a>
> +  <a href="enter_bug.cgi?format=__standard__&product=[% product.name FILTER uri %]">

s/&/&amp;/

Comment 10

[Gervase Markham [:gerv]]

(In reply to Byron Jones ‹:glob› from comment #9)
> this mostly looks good.
> don't forget to test your code, and run bugzilla's tests.

I did test my code (no idea how it still works with that XXX in there!) but forgot Bugzilla's tests.

> the url which http://mzl.la/16KuCao points to isn't valid - form.web-compat
> isn't valid even with this patch.

Oops - I can make a new one.

> i would prefer to see the full url here, rather than hiding it behind a
> shortener:
> <a
> href="https://bugzilla.mozilla.org/form.mobile.compat">bugzilla.mozilla.org/
> form.mobile.compat</a>
> while longer, it would be easier to enter into a mobile device when compared
> to the mix of random digits and letters from mzl.la.

I'd like to get a mzl.la/somethingsensible, but don't know how to do it.

> > +[ <span class="required_star">*</span> <span class="required_explanation">Required Field</span> ]
> > +
> > +<div id="standard_link">
> > +  <a href="enter_bug.cgi?format=__standard__&product=[% product.name FILTER uri %]">
> 
> s/&/&amp;/

These bugs also exist in the IPP template, then :-)

New patch soon.

Gerv

Comment 11

[:glob ✱]

(In reply to Gervase Markham [:gerv] from comment #10)
> > the url which http://mzl.la/16KuCao points to isn't valid - form.web-compat
> > isn't valid even with this patch.
> 
> Oops - I can make a new one.

no need - please use the full url as per my review.

Comment 12

[Karl Dubost💡 :karlcow]

We have an issue for small devices. Bugzilla is not usable. See Bug 101865 I left a comment there.

Comment 13

[Gervase Markham [:gerv]]

(In reply to Anthony Ricaud (:rik) from comment #8)
> Karl asked for my comments on this:
> - For metrics purpose, it might be useful to set a keyword/whiteboard/flag
> for bugs entered through this form.

What would you do with such information?

> - Even if the form is simplified, the wording is not very engaging. Some
> suggestions:
>   * "Steps to reproduce" -> "Describe what you were doing"

We want a series of steps, because experience shows that getting people thinking in that way leads to much better bug reports. (This bug report is modelled on other successful 'simple' bug reporting forms.)

>   * "Actual result" -> "What happened?"
>   * "Expected result" -> "What were you expecting instead?"
>   And I'd place "What happened?" before "Describe what you were doing".

Again, getting people thinking logically stepwise is much more likely to get a bug report with all the info. If the first question is "What happened?", you are likely to get the answer "The site didn't work".

Gerv

Comment 14

[Gervase Markham [:gerv]]

Attachment: 904713.diff

Thanks for the previous quick review :-) Review comments addressed.

Gerv

Comment 15

[David Lawrence [:dkl]]

Comment on attachment 790237 [details] [diff] [review]
904713.diff

Review of attachment 790237 [details] [diff] [review]:
-----------------------------------------------------------------

Close. Just a couple small issues.

dkl

::: extensions/BMO/template/en/default/bug/create/create-mobile-compat.html.tmpl
@@ +92,5 @@
> +  <input type="hidden" name="user_agent"   value="[% cgi.user_agent() FILTER html %]">
> +
> +  <input type="hidden" name="token"        value="[% token FILTER html %]">
> +
> +[% IF NOT cgi.user_agent("Mobile") %]

Doesn't this need to be a string search and not a comparison on "Mobile"? The useragent string will contain more than just Mobile.

@@ +140,5 @@
> +
> +<tr>
> +  <th class="required">Steps To Reproduce</th>
> +  <td>
> +    <textarea id="desc" name="desc" cols="50" rows="5">1.

You have this field marked as required but you are not checking in the javascript that the field is not empty. Someone can remove the contents. Also you will need to make sure the string does not equal '1.\n2.\n3.\n...' to truly make sure they entered actual steps.

Comment 16

[Gervase Markham [:gerv]]

(In reply to David Lawrence [:dkl] from comment #15)
> Doesn't this need to be a string search and not a comparison on "Mobile"?
> The useragent string will contain more than just Mobile.

I know :-) I'm taking my cue from the Guided form, which does:

[% matches = cgi.user_agent('Gecko/(\d+)') %]

That suggests to me that this method takes a regexp substring match. I tested it on my desktop browser both ways (adding and removing the NOT) and it seems to work fine. It certainly shows the message on my desktop browser.

> You have this field marked as required but you are not checking in the
> javascript that the field is not empty. Someone can remove the contents.
> Also you will need to make sure the string does not equal '1.\n2.\n3.\n...'
> to truly make sure they entered actual steps.

This is noted in my "limitations" in comment 2, bullet 4. Given that this is a guide to users and not a security issue, I think this will do for version 1.

So r+? :-)

Gerv

Comment 17

[David Lawrence [:dkl]]

Comment on attachment 790237 [details] [diff] [review]
904713.diff

(In reply to Gervase Markham [:gerv] from comment #16) 
> I know :-) I'm taking my cue from the Guided form, which does:
> 
> [% matches = cgi.user_agent('Gecko/(\d+)') %]
> 
> That suggests to me that this method takes a regexp substring match. I
> tested it on my desktop browser both ways (adding and removing the NOT) and
> it seems to work fine. It certainly shows the message on my desktop browser.

Yeah I realized the error of my comment later while driving :) cgi.user_agent does in fact do a regexp match so your statement is proper. I was improperly reading it as like cgi.param in that it had to match exactly.

> This is noted in my "limitations" in comment 2, bullet 4. Given that this is
> a guide to users and not a security issue, I think this will do for version
> 1.

Ah I see now. I would still simply add 'desc' to your field_errors data which will at least check if they cleared the textarea by mistake.

r=dkl
 
dkl

Comment 18

[Gervase Markham [:gerv]]

Attachment: Patch v.3

Add "desc" to error array. Carrying forward r+. 

Please can this be checked in in time for this week's BMO push?

Thanks,

Gerv

Comment 19

[David Lawrence [:dkl]]

Committing to: bzr+ssh://[e-mail]/bmo/4.2
modified .htaccess
added extensions/BMO/template/en/default/bug/create/comment-mobile-compat.txt.tmpl
added extensions/BMO/template/en/default/bug/create/create-mobile-compat.html.tmpl
Committed revision 8944.

Comment 20

[Gervase Markham [:gerv]]

Thank you :-)

Gerv

Comment 21

[Gervase Markham [:gerv]]

This is now available at:
https://bugzilla.mozilla.org/form.mobile.compat

Gerv