All buttons should have an ID

RESOLVED FIXED in Bugzilla 4.2

Status

()

Bugzilla
User Interface
--
minor
RESOLVED FIXED
10 years ago
7 years ago

People

(Reporter: Frédéric Buclin, Assigned: rojanu)

Tracking

Bugzilla 4.2
Bug Flags:
approval +

Details

Attachments

(1 attachment, 1 obsolete attachment)

7.53 KB, patch
Frédéric Buclin
: review+
Details | Diff | Splinter Review
(Reporter)

Description

10 years ago
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">
(Reporter)

Comment 1

9 years ago
The "Add" button when adding new classifications has no ID either.
(Reporter)

Updated

7 years ago
Whiteboard: [Good Intro Bug]
(Assignee)

Comment 2

7 years ago
Created attachment 516239 [details] [diff] [review]
buton_id_fix
Attachment #516239 - Flags: review?(LpSolit)
(Reporter)

Comment 3

7 years ago
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?
Attachment #516239 - Flags: review?(LpSolit) → review+
(Reporter)

Comment 4

7 years ago
Holding approval per my previous comment.
Assignee: ui → aliustek
Status: NEW → ASSIGNED
Flags: approval?
Whiteboard: [Good Intro Bug]
Target Milestone: --- → Bugzilla 4.2
(Assignee)

Comment 5

7 years ago
Created attachment 517855 [details] [diff] [review]
Button_id_fix_2

Updated patch according to comments
Attachment #516239 - Attachment is obsolete: true
Attachment #517855 - Flags: review?(LpSolit)
(Reporter)

Comment 6

7 years ago
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
Attachment #517855 - Flags: review?(LpSolit) → review+
(Reporter)

Updated

7 years ago
Flags: approval? → approval+
(Reporter)

Comment 7

7 years ago
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/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
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.