1050226 - Offer web devs a more focused set of product/components to file bugs in

Status: RESOLVED FIXED

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

Description

[:Gijs (he/him)]

We could probably ask something like:
"Are you reporting a bug with:
- HTML, CSS, JS, SVG, or some other web technology or combination of web
technologies?
- Firefox's developer tools
- Firefox's user interface (for example, an issue with bookmarks, tabbed
browsing or the location bar)
"

and automatically select the right product/component for them (being
Core::Untriaged, Firefox::Developer Tools, and Firefox::Untriaged,
respectively).



(adapted from https://groups.google.com/forum/#!topic/mozilla.tools.bmo/RJYPz3ax9z4 based on feedback from Zack Weinberg)

Comment 1

[Dylan Hardison [:dylan] (he/him)]

Context (from earlier offline discussion)

> Will need to add an optional parameter to guided bug entry, mode=webdev.
> When that is passed, the initial step in guided.js is set to "webdev" instead from "product".
> The webdev step will show the question listed in the bug, set the product and skip to the step that
> comes after "product" normally.

Status update:

1) Added "webdev" step which is the default step when ?webdev=1 is passed. (So this will not be directly accessible until the other parts of guided bug entry land)
2) Adding a step to the current system requires a javascript object that implements an .onShow and .onInit() at minimum. I've basically copied the 'product' step to do this. Added a step div to the template. After the webdev.onShow() happens, the next step is 'dupes'. 

Still need to remove the (now quite useless) step counter, fix js (this file uses 2-space indent...).
Might need to do something to the "show advanced form" link during the webdev step.

Comment 2

[Dylan Hardison [:dylan] (he/him)]

Attachment: bug-1050226-v1.patch

This ended up being both harder to do, but easier to implement than I had imagined. Originally I thought I would need hooks in webdev.onShow() to set the default step back to product, and that when a ?webdev=1 query parameter was added, set the default first step to "webdev". However, this is wrong. It is entirely possible to just navigate to the form with #h=webdev appended to the end.

Meanwhile, such navigation does not work if you use <a href="#" onclick="blah" links (duh), which is why the existing code uses href="javascript:void(0)". This is probably sub-optimal.

Also, the current way that all steps for the guided bug process are in one file is probably not going to scale if we add a lot more custom steps in the future.

Note, in testing you'll need to navigate to enter_bug.cgi?format=guided#h=webdev, and make sure Firefox::Untriaged, Core::Untriaged, and Firefox::Developer Tools exist.
At least that last one had to be created on my box.

Comment 3

[Dylan Hardison [:dylan] (he/him)]

Attachment: bug-1050226-v1.1.patch

woops. I hadn't added back the generic product selector at the bottom -- which I think should be there.

Comment 4

[:glob ✱]

Comment on attachment 8513545 [details] [diff] [review]
bug-1050226-v1.1.patch

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

t/009bugwords.t ...... 1/627
#   Failed test 'extensions/GuidedBugEntry/template/en/default/guided/guided.html.tmpl contains invalid bare words (e.g. 'bug') --WARNING'

the bugs are not filed into the 'developer tools' component when selecting the second option, most likely due to the 'defaultComponent' being set in products.js.

::: extensions/GuidedBugEntry/template/en/default/pages/guided_products.js.tmpl
@@ +8,5 @@
>  
>  [%# this file allows us to pull in data defined in the BMO ext %]
>  
> +[% IF webdev %]
> +guided.webdev = true;

nit: indent

Comment 5

[Dylan Hardison [:dylan] (he/him)]

(In reply to Byron Jones ‹:glob› from comment #4)
> Comment on attachment 8513545 [details] [diff] [review]
> bug-1050226-v1.1.patch
> 
> Review of attachment 8513545 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> t/009bugwords.t ...... 1/627
> #   Failed test
> 'extensions/GuidedBugEntry/template/en/default/guided/guided.html.tmpl
> contains invalid bare words (e.g. 'bug') --WARNING'
> 
> the bugs are not filed into the 'developer tools' component when selecting
> the second option, most likely due to the 'defaultComponent' being set in
> products.js.

Defeating the default component logic was easy, if I always defeat it. It looks like I'm going to need the ?webdev=1 query string param after all. Quite frustrating!

Comment 6

[Dylan Hardison [:dylan] (he/him)]

Attachment: bug-1050226-v2.patch

Revised patch, defaults to the right component.
Note I've noticed some unrelated odd behavior with the component selection drop down, but not sure if that is a bug.

Comment 7

[Dylan Hardison [:dylan] (he/him)]

Attachment: bug-1050226-v2.1.patch

Fixed typo

Comment 8

[:glob ✱]

Comment on attachment 8520003 [details] [diff] [review]
bug-1050226-v2.1.patch

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

this is better, however..

- we shouldn't allow the component to be changed on the last step
- clicking on 'change' from the last step takes you back to 'product' step, however it should take you to the webdev step

::: extensions/GuidedBugEntry/template/en/default/guided/guided.html.tmpl
@@ +112,5 @@
> +  <li><a href="javascript:void(0)"
> +         onclick="product.select('Firefox', 'Untriaged');">
> +         Firefox's user interface (for example, an issue with bookmarks,
> +         tabbed browsing or the location bar)</a>
> +  </li>

i'm thinking these options could do with some icons to make them stand out.
i suggest:

html/css/js/svg : use the "Core" icon
dev tools : use the devedition firefox icon (https://db.tt/clqtPzhr)
ui : use the normal firefox icon

::: extensions/GuidedBugEntry/web/js/guided.js
@@ +204,4 @@
>      }
>  
>      // show/hide component selection row
> +    if (products[productName] && products[productName].noComponentSelection && !guided.webdev) {

would it be possible/viable for the webdev step to instead override/set the product's defaultComponent?

if it's too fiddly, continuing with this approach is ok.

Comment 9

[Dylan Hardison [:dylan] (he/him)]

(In reply to Byron Jones ‹:glob› from comment #8)
> Comment on attachment 8520003 [details] [diff] [review]
> bug-1050226-v2.1.patch
> 
> Review of attachment 8520003 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> this is better, however..
> 
> - we shouldn't allow the component to be changed on the last step
> - clicking on 'change' from the last step takes you back to 'product' step,
> however it should take you to the webdev step
Ah! right, I meant to detect webdevness there and redirect to the webdev step.

> ::: extensions/GuidedBugEntry/template/en/default/guided/guided.html.tmpl
> @@ +112,5 @@
> > +  <li><a href="javascript:void(0)"
> > +         onclick="product.select('Firefox', 'Untriaged');">
> > +         Firefox's user interface (for example, an issue with bookmarks,
> > +         tabbed browsing or the location bar)</a>
> > +  </li>
> 
> i'm thinking these options could do with some icons to make them stand out.
> i suggest:
> 
> html/css/js/svg : use the "Core" icon
> dev tools : use the devedition firefox icon (https://db.tt/clqtPzhr)
> ui : use the normal firefox icon

Ah, yes. Should each list item be more boxy as a result?

> ::: extensions/GuidedBugEntry/web/js/guided.js
> @@ +204,4 @@
> >      }
> >  
> >      // show/hide component selection row
> > +    if (products[productName] && products[productName].noComponentSelection && !guided.webdev) {
> 
> would it be possible/viable for the webdev step to instead override/set the
> product's defaultComponent?
> 
> if it's too fiddly, continuing with this approach is ok.
It is a little fiddly, I tried doing that before. When I'm working #1 above I'll see what can be done.
(it does feel crufty right now).

Comment 10

[Dylan Hardison [:dylan] (he/him)]

This feature now depends on the improvements made in bug 1050232, so that the questions asked can be shown in little boxes.

Comment 11

[Dylan Hardison [:dylan] (he/him)]

Attachment: bug-1050226-v3.patch

With product icons, component non-editable and other enhancements. I also removed some suspected dead code (exit_block) in the templates.

Comment 12

[:glob ✱]

Comment on attachment 8527244 [details] [diff] [review]
bug-1050226-v3.patch

as this relies on bug 1050232, i'm clearing this review until that bug has an r+'d patch.

Comment 13

[Dylan Hardison [:dylan] (he/him)]

Attachment: bug-1050226-v4.patch

To review, apply after Bug 1050232.

No changes, just rebased on current Bug 1050232,

Comment 14

[Dylan Hardison [:dylan] (he/him)]

Right! I was also going to mention: I sent this in for review before the r+ of the other patch so that one doesn't block the other. Mark thought it was a good idea. :-)

Comment 15

[:glob ✱]

Comment on attachment 8537867 [details] [diff] [review]
bug-1050226-v4.patch

as this relies on bug 1050232, i'm clearing this review until that bug has an r+'d patch.

Comment 16

[:glob ✱]

Comment on attachment 8537867 [details] [diff] [review]
bug-1050226-v4.patch

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

the iconified page looks much better, great work :)

however looks like the fixes for these two issues were missed in this revision:
- we shouldn't allow the component to be changed on the last step
- clicking on 'change' from the last step takes you back to 'product' step, however it should take you to the webdev step

::: extensions/GuidedBugEntry/template/en/default/guided/guided.html.tmpl
@@ +269,4 @@
>        </div>
>      </td>
>      <td class="exit_text_last">
> +      <a href="enter_bug.cgi?format=guided&amp;webdev=1">Report an issue with Firefox on a site that I've developed</a><br>

this should probably take you direct to core|untraiged

it seems odd for a link that says it's for reporting issues with a site then then ask if they want to report a bug in the devtools or firefox ui.

::: extensions/GuidedBugEntry/web/js/guided.js
@@ +133,5 @@
> +    if (componentName) {
> +      this.setPreselectedComponent(componentName);
> +      if (prod && prod.defaultComponent) {
> +        prod.originalDefaultComponent = prod.originalDefaultComponent || prod.defaultComponent;
> +        prod.defaultComponent = componentName; 

nit: there be a trailing space here.

Comment 17

[Dylan Hardison [:dylan] (he/him)]

Hmm, I remember specifically addressing "we shouldn't allow the component to be changed on the last step" before, but you're right, that is not done here.

Now that the UI-changing patch has landed, I can clean this one cleaned up properly.

Comment 18

[Dylan Hardison [:dylan] (he/him)]

(In reply to Byron Jones ‹:glob› (limited availability until 19th jan) from comment #16)
> Comment on attachment 8537867 [details] [diff] [review]
> bug-1050226-v4.patch
>
> however looks like the fixes for these two issues were missed in this
> revision:
> - we shouldn't allow the component to be changed on the last step
In all cases? I have verified that the *existing* behavior (with respect to noComponentSelection) is still in place. That is, if a product in products.js has noComponentSelection, no component can be selected (by the user). Did you encounter something different than this?

Screenshot to follow

Comment 19

[Dylan Hardison [:dylan] (he/him)]

Attachment: last-step.png

Comment 20

[Dylan Hardison [:dylan] (he/him)]

Attachment: bug-1050226-v5.patch

This works for me -- making setStep('default') be the action of the "Change" links. This results in the default step being selected, which is changed via the webdev mode already.

Comment 21

[:glob ✱]

(In reply to Dylan William Hardison [:dylan] from comment #18)
> > - we shouldn't allow the component to be changed on the last step
> In all cases?

depends on what you mean by "in all cases" :)

> I have verified that the *existing* behavior (with respect to
> noComponentSelection) is still in place. That is, if a product in
> products.js has noComponentSelection, no component can be selected (by the
> user). Did you encounter something different than this?

from comment 0:
> and automatically select the right product/component for them (being
> Core::Untriaged, Firefox::Developer Tools, and Firefox::Untriaged,
> respectively).

currently selecting "devtools" or "firefox ui" results in the component select being hidden, however selecting "web technologies" does not.  selecting any of the three web-dev options should pre-select and hide the component select.

the behaviour of non-web-dev shouldn't change.

Comment 22

[:glob ✱]

Comment on attachment 8545486 [details] [diff] [review]
bug-1050226-v5.patch

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

as per comment 21, the component select needs to be hidden.

the other changes look good, except i get a weird syntax error when selecting the "report an issue with a website.." option:
SyntaxError: expected expression, got ')'

Comment 23

[Dylan Hardison [:dylan] (he/him)]

(In reply to Byron Jones ‹:glob› (limited availability until 19th jan) from comment #21)
> (In reply to Dylan William Hardison [:dylan] from comment #18)
> > > - we shouldn't allow the component to be changed on the last step
> > In all cases?
> 
> depends on what you mean by "in all cases" :)
> 
> > I have verified that the *existing* behavior (with respect to
> > noComponentSelection) is still in place. That is, if a product in
> > products.js has noComponentSelection, no component can be selected (by the
> > user). Did you encounter something different than this?
> 
> from comment 0:
> > and automatically select the right product/component for them (being
> > Core::Untriaged, Firefox::Developer Tools, and Firefox::Untriaged,
> > respectively).
> 
> currently selecting "devtools" or "firefox ui" results in the component
> select being hidden, however selecting "web technologies" does not. 
> selecting any of the three web-dev options should pre-select and hide the
> component select.
> 
> the behaviour of non-web-dev shouldn't change.
I will have to shadow the noComponentSelection when in webdev mode, I suppose, so any webdev component is "fixed" in webdev mode but not outside of it.

Comment 24

[Dylan Hardison [:dylan] (he/him)]

I've been in a style hole, for no other reason that I realized the alternating row colors is broken when component selection is hidden. If that unsightliness is acceptable I may just go ahead with it (it's hidden for all components if and only if webdev mode is on) and worry about redoing the bugform in a more modern, HTML5-y way later.

Comment 25

[Mark Côté [:mcote]]

Maybe throw it up on bugzilla-dev so we can see how ugly it may or may not be?

Comment 26

[Zack Weinberg (:zwol)]

(In reply to Mark Côté [:mcote] from comment #25)
> Maybe throw it up on bugzilla-dev so we can see how ugly it may or may not
> be?

I'd appreciate being able to poke at a test install myself -- not being familiar with the guts of Bugzilla, it's almost impossible for me to tell what the end result will be like from the patches.

Comment 27

[Dylan Hardison [:dylan] (he/him)]

(In reply to Zack Weinberg (:zwol) from comment #26)
> (In reply to Mark Côté [:mcote] from comment #25)
> > Maybe throw it up on bugzilla-dev so we can see how ugly it may or may not
> > be?
> 
> I'd appreciate being able to poke at a test install myself -- not being
> familiar with the guts of Bugzilla, it's almost impossible for me to tell
> what the end result will be like from the patches.

That's a good idea.

Comment 28

[Dylan Hardison [:dylan] (he/him)]

Code (finally) up on dev. Here's a link to the form in question. This is normally the last step in the guided bug process.

https://bugzilla-dev.allizom.org/enter_bug.cgi?format=guided&webdev=1#h=bugForm|Firefox|Developer+Tools

Comment 29

[Dylan Hardison [:dylan] (he/him)]

Attachment: bug-1050226-v6.patch

Comment 30

[Dylan Hardison [:dylan] (he/him)]

Zack, what do you think of the form in the final step? Is the even/odd row coloring (which is broken by hiding a row) worth fixing now, or should it remain until the form can be overhauled?

Comment 31

[Zack Weinberg (:zwol)]

(In reply to Dylan William Hardison [:dylan] from comment #30)
> Zack, what do you think of the form in the final step? Is the even/odd row
> coloring (which is broken by hiding a row) worth fixing now, or should it
> remain until the form can be overhauled?

If you're talking about what I think you're talking about (the "product" row having the same background as the "What did you do?" row), it's a little weird but I wouldn't worry about it right now, considering that the form itself is still presenting the same generic steps-to-reproduce template that is a poor fit for webdev Core bugs.  I think fixing that is much more important than a cosmetic issue with the background colors.

I'm not sure I would use even/odd row coloring for this form at all -- I don't think it works especially well, especially the way the large textareas are given a different color than their prompts -- but I'm not a graphic designer.

(As long as we're talking graphical nits, can you make the "Clear" button and "File description" row hidden when no file has been attached?  That's orthogonal, but seems like it would be an improvement across the board.)

Comment 32

[:glob ✱]

Comment on attachment 8546833 [details] [diff] [review]
bug-1050226-v6.patch

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

dropping the background banding makes sense :)  #E0E0E0 for all rows looks nice.

> can you make the "Clear" button and "File description" row hidden when no file has been attached?
+1

soft r- to address these nits.

::: extensions/GuidedBugEntry/template/en/default/guided/guided.html.tmpl
@@ +53,4 @@
>  
>  <script type="text/javascript">
>  YAHOO.util.Dom.addClass('loading', 'hidden');
> +guided.init({ webdev: [% IF webdev; GET "true"; ELSE; GET "false"; END %] });

[% webdev ? "true" : "false" %]

Comment 33

[Dylan Hardison [:dylan] (he/him)]

Attachment: bug-1050226-v7.patch

Changes per soft r- -- unsure if that meant "commit after fixing" or if another review cycle should be done.

Comment 34

[:glob ✱]

(In reply to Dylan William Hardison [:dylan] from comment #33)
> Changes per soft r- -- unsure if that meant "commit after fixing" or if
> another review cycle should be done.

an r- always means another review cycle :)

Comment 35

[:glob ✱]

Comment on attachment 8554825 [details] [diff] [review]
bug-1050226-v7.patch

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

r=glob

Comment 36

[Dylan Hardison [:dylan] (he/him)]

I realize this is not discoverable until bug 1050224 is complete. Should it still be commited?

Comment 37

[:glob ✱]

(In reply to Dylan William Hardison [:dylan] from comment #36)
> I realize this is not discoverable until bug 1050224 is complete. Should it
> still be commited?

yes.  it will allow us to give the direct url to people who want to both file firefox bugs and help test bmo :)

Comment 38

[Dylan Hardison [:dylan] (he/him)]

To ssh://[e-mail]/webtools/bmo/bugzilla.git
   1ccc683..2344279  master -> master