Closed Bug 1200765 Opened 10 years ago Closed 10 years ago

Need to do this for BMO first. The changes to upstream bugzilla will be a bit different due to divergence, and we need this in BMO soon for the 2fa change that makes api keys more of a requirement.

Tested in Firefox for Android, chrome for android, and the Responsive view in firefox desktop. The solution is generalized a bit because we might want the same treatment for error pages. "Request Desktop Site" is honored.

Summary: Make oauthy type UX mobile friendly → Make login UX mobile friendly to assist mobile authentication workflow

Comment on attachment 8656205 [details] [diff] [review] 1200765_1.patch Review of attachment 8656205 [details] [diff] [review]: ----------------------------------------------------------------- the screen is too wide for iphone 5 and earlier (320 x 568). we should also increase the font size on mobile. ::: .htaccess @@ +91,4 @@ > RewriteRule ^(?:latest|1\.2|1\.3)/(.*)$ extensions/BzAPI/bin/rest.cgi/$1 [NE] > RewriteRule ^bzapi/(.*)$ extensions/BzAPI/bin/rest.cgi/$1 [NE] > RewriteRule ^data/assets/ZeroClipboard.swf(.*)$ extensions/BugModal/web/ZeroClipboard/ZeroClipboard.swf$1 [NE] > +RewriteRule ^login$ index.cgi?GoAheadAndLogIn=1 [NE] this doesn't appear to be used. ::: Bugzilla/Template.pm @@ +1124,5 @@ > + 'is_mobile_browser' => sub { > + my $cgi = Bugzilla->cgi; > + > + return 1; > + return $cgi->user_agent =~ /Mobi/; this will always return true (debugging i guess). nit: no need to create the $cgi var here ::: skins/custom/mobile.css @@ +1,1 @@ > +/* This Source Code Form is subject to the terms of the Mozilla Public as this is part of the core header move this from /custom/ to /standard/

This looks pretty reasonable on an assortment of phones (and emulations of phones).

Comment on attachment 8657989 [details] [diff] [review] 1200765_3.patch Review of attachment 8657989 [details] [diff] [review]: ----------------------------------------------------------------- looks good on mobile, but there's some regressions on desktop that should be fixed. ::: skins/custom/mobile.css @@ +25,5 @@ > + > + body { > + font-size: normal; > + } > +} \ No newline at end of file this file appears to be redundant and should be deleted. ::: template/en/default/account/auth/login.html.tmpl @@ +44,5 @@ > +<div id="login" class="login-form"> > + <form name="login" action="[% target FILTER html %]" method="POST" > + [%- IF Bugzilla.cgi.param("data") %] enctype="multipart/form-data"[% END %]> > + <div class="field-login"> > + <label for="Bugzilla_login">Email Address:</label> the labels should be bold, and vertically centered @@ +46,5 @@ > + [%- IF Bugzilla.cgi.param("data") %] enctype="multipart/form-data"[% END %]> > + <div class="field-login"> > + <label for="Bugzilla_login">Email Address:</label> > + <input id="Bugzilla_login" name="Bugzilla_login" > + [%- ' type="email"' UNLESS Param('emailsuffix') %]> the _login and _password fields are touching - add a gap to avoid this @@ +52,5 @@ > + </div> > + > + <div class="field-password"> > + <label for="Bugzilla_password">Password:</label> > + <input type="password" id="Bugzilla_password" name="Bugzilla_password"> on desktop the width of the fields is now smaller. we have quite a few long email addresses so this now looks weird. i suggest a width of 20em for both fields on non-mobile displays. @@ +62,5 @@ > + <input type="checkbox" id="Bugzilla_remember" name="Bugzilla_remember" value="on" > + [%+ "checked" IF Param('rememberlogin') == "defaulton" %]> > + <label for="Bugzilla_remember" class="checkbox-note"> > + Remember my email address > + </label> on desktop -remember and -restrict should have a 7em left margin to match the current layout @@ +80,5 @@ > > + <div class="field-submit"> > + <input type="hidden" name="Bugzilla_login_token" > + value="[% get_login_request_token() FILTER html %]"> > + <input type="submit" name="GoAheadAndLogIn" value="Sign in" id="log_in"> you've changed "Log in" to "Sign in" - revert this change (we use "Log in" consistently).

aligned the fields, added padding, etc.

The duo CSS for the iframe looks simple but as duo hasn't been reviewed yet, I have not included it in this patch. This changes the totp elements to be bigger and adds allow_mobile to the verify-totp template.

(In reply to Dylan William Hardison [:dylan] from comment #8) > The duo CSS for the iframe looks simple but as duo hasn't been reviewed yet, > I have not included it in this patch. oh yeah .. i've been in duo land for so long i forgot it isn't in the tree yet :)

Status: NEW → RESOLVED

Closed: 10 years ago

Resolution: --- → FIXED