Last Comment Bug 850126 - token id defined twice on logged out pages
: token id defined twice on logged out pages
Status: RESOLVED FIXED
:
Product: Bugzilla
Classification: Server Software
Component: User Interface (show other bugs)
: 4.2
: All All
: -- trivial (vote)
: Bugzilla 4.2
Assigned To: Reed Loden [:reed] (use needinfo?)
: default-qa
:
Mentors:
Depends on: 706271
Blocks:
  Show dependency treegraph
 
Reported: 2013-03-11 21:20 PDT by Byron Jones ‹:glob› [PTO until 2016-10-10]
Modified: 2013-03-12 10:09 PDT (History)
3 users (show)
LpSolit: approval+
LpSolit: approval4.4+
LpSolit: approval4.2+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch - v1 (746 bytes, patch)
2013-03-11 21:32 PDT, Reed Loden [:reed] (use needinfo?)
LpSolit: review+
Details | Diff | Splinter Review

Description Byron Jones ‹:glob› [PTO until 2016-10-10] 2013-03-11 21:20:49 PDT
the hidden field named "token", with id "token", is defined twice for logged out users - once in forgot_form_top and again in forgot_form_bottom.
Comment 1 Reed Loden [:reed] (use needinfo?) 2013-03-11 21:32:09 PDT
Created attachment 723803 [details] [diff] [review]
patch - v1

untested
Comment 2 Byron Jones ‹:glob› [PTO until 2016-10-10] 2013-03-11 21:45:46 PDT
Comment on attachment 723803 [details] [diff] [review]
patch - v1

if the id attribute is used, this patch would break things.
if the id attribute isn't used, it would be simpler to just remove it from the field rather than renaming it.
Comment 3 Frédéric Buclin 2013-03-12 04:32:44 PDT
Comment on attachment 723803 [details] [diff] [review]
patch - v1

No, it should stay, but renamed. Without an ID, you would have no chance to correctly identify this field as you would have several fields with the same name (automated QA tests!). r=LpSolit
Comment 4 Frédéric Buclin 2013-03-12 04:34:19 PDT
Waiting for glob to withdraw his r-. :)
Comment 5 Byron Jones ‹:glob› [PTO until 2016-10-10] 2013-03-12 07:48:48 PDT
Comment on attachment 723803 [details] [diff] [review]
patch - v1

(In reply to Frédéric Buclin from comment #3)
> No, it should stay, but renamed. Without an ID, you would have no chance to
> correctly identify this field as you would have several fields with the same
> name (automated QA tests!). r=LpSolit

this isn't true, you can reference the tokens via the form:
document.getElementById('forgot_form_top')['token']
document.getElementById('forgot_form_bottom')['token']

but if it makes your life easier, and doesn't break existing functionality, i'm happy to withdraw my r-
Comment 6 Frédéric Buclin 2013-03-12 08:47:12 PDT
(In reply to Byron Jones ‹:glob› from comment #5)
> this isn't true, you can reference the tokens via the form:
> document.getElementById('forgot_form_top')['token']

FYI, our Selenium scripts are written in Perl, not JS. ;) And your code is heavily dependent on the DOM of the page, which makes it rather fragile. That's why I opened bug 317694 and bug 449067 some years ago, to make our life easier when writing automated tests.
Comment 7 Reed Loden [:reed] (use needinfo?) 2013-03-12 10:09:01 PDT
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified template/en/default/account/auth/login-small.html.tmpl
Committed revision 8589.

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/4.4/
modified template/en/default/account/auth/login-small.html.tmpl
Committed revision 8529.

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/4.2/
modified template/en/default/account/auth/login-small.html.tmpl
Committed revision 8196.

Note You need to log in before you can comment on or make changes to this bug.