449067 - All buttons should have an ID

Status: RESOLVED FIXED

(Bugzilla :: User Interface, defect)

Description

[Frédéric Buclin]

Read bug 317694 for why all buttons should have an ID (easier to use with Selenium). The buttons below have no ID.

admin/flag-type/edit.html.tmpl:

<input type="submit" name="categoryAction-include" value="Include">
<input type="submit" name="categoryAction-exclude" value="Exclude">
<input type="submit" name="categoryAction-removeInclusion" value="Remove Inclusion">
<input type="submit" name="categoryAction-removeExclusion" value="Remove Exclusion">


admin/workflow/edit.html.tmpl:

<input type="submit" value="Commit Changes">


admin/workflow/comment.html.tmpl:

<input type="submit" value="Commit Changes">


admin/groups/edit.html.tmpl:

<input type="submit" value="Update Group">
<input type="submit" value="Remove Memberships">


admin/groups/confirm-remove.html.tmpl:

<input name="confirm" type="submit" value="Confirm">


admin/products/edit.html.tmpl:

<input type="submit" name="submit" value="Save Changes">


admin/products/create.html.tmpl:

<input type="submit" value="Add">


admin/params/editparams.html.tmpl:

<input type="submit" name="action" value="Save Changes">


admin/sudo.html.tmpl:

<input type="submit" value="Begin Session">


search/search-create-series.html.tmpl:

<input type="submit" name="action-search" value="Run Search">

Comment 1

[Frédéric Buclin]

The "Add" button when adding new classifications has no ID either.

Comment 2

[rojanu]

Attachment: buton_id_fix

Comment 3

[Frédéric Buclin]

Comment on attachment 516239 [details] [diff] [review]
buton_id_fix

>=== modified file 'template/en/default/admin/flag-type/edit.html.tmpl'

>+              <input type="submit" id="categoryAction-include" name="categoryAction-include" value="Include">
>+              <input type="submit" id="categoryAction-exclude" name="categoryAction-exclude" value="Exclude">

These two lines are a bit too long. They should be split.


>+              <input type="submit" id="categoryAction-removeInclusion" name="categoryAction-removeInclusion" value="Remove Inclusion">
>+              <input type="submit" id="categoryAction-removeExclusion" name="categoryAction-removeExclusion" value="Remove Exclusion">

Same here.



>=== modified file 'template/en/default/admin/params/editparams.html.tmpl'

>           <input type="hidden" name="action" value="save">
>-          <input type="submit" name="action" value="Save Changes">

Unrelated to your patch, but this is a bug. We shouldn't have two fields with the same name. editparams.cgi doesn't expect to get two different values. The button should have no name.


>+          <input type="submit" id="action" name="action" value="Save Changes">

Per my comment above, we should change the ID and drop the name here.



>=== modified file 'template/en/default/admin/products/edit.html.tmpl'

>-  <input type="submit" name="submit" value="Save Changes">
>+  <input type="submit" id="update-product" name="submit" value="Save Changes">

The ID and name should ideally match, when both present. Here, it doesn't make sense to define the name. It's a button, and that's the single one in this form.


r=LpSolit, but would you agree to address my comments above and re-attach an updated patch, please?

Comment 4

[Frédéric Buclin]

Holding approval per my previous comment.

Comment 5

[rojanu]

Attachment: Button_id_fix_2

Updated patch according to comments

Comment 6

[Frédéric Buclin]

Comment on attachment 517855 [details] [diff] [review]
Button_id_fix_2

>=== modified file 'template/en/default/admin/flag-type/edit.html.tmpl'

>+              <input type="submit" id="categoryAction-include" 
>+                                 name="categoryAction-include" value="Include">
>+              <input type="submit" id="categoryAction-exclude"
>+                                 name="categoryAction-exclude" value="Exclude">

The indentation is a bit weird, but this can be fixed on checkin.


>+              <input type="submit" id="categoryAction-removeInclusion" 
>+                 name="categoryAction-removeInclusion" value="Remove Inclusion">
>+              <input type="submit" id="categoryAction-removeExclusion" 
>+                 name="categoryAction-removeExclusion" value="Remove Exclusion">

Same here.


Otherwise looks good. Thanks for the patch! :) r=LpSolit

Comment 7

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified template/en/default/admin/sudo.html.tmpl
modified template/en/default/admin/flag-type/edit.html.tmpl
modified template/en/default/admin/groups/confirm-remove.html.tmpl
modified template/en/default/admin/groups/edit.html.tmpl
modified template/en/default/admin/params/editparams.html.tmpl
modified template/en/default/admin/products/create.html.tmpl
modified template/en/default/admin/products/edit.html.tmpl
modified template/en/default/admin/workflow/comment.html.tmpl
modified template/en/default/admin/workflow/edit.html.tmpl
modified template/en/default/search/search-create-series.html.tmpl
revision 7761