Issue 10440061: Remaining style fixes from http://codereview.chromium.org/10443024/.

Issue 10440061: Remaining style fixes from http://codereview.chromium.org/10443024/. (Closed)

Created:
8 years, 7 months ago by zel

Modified:
8 years, 7 months ago

Reviewers:
csilv

CC:
chromium-reviews, arv (Not doing code reviews)

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

More Reviews

Description

Remaining style fixes from http://codereview.chromium.org/10443024/. BUG=129644 TEST=none Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=139158

Patch Set 2 : #

Created: 8 years, 7 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+34 lines, -47 lines)			Patch
M	chrome/browser/resources/gaia_auth/main.css	View		1 chunk	+1 line, -1 line	0 comments	Download
M	chrome/browser/resources/gaia_auth/main.html	View		1 chunk	+6 lines, -2 lines	0 comments	Download
M	chrome/browser/resources/gaia_auth/main.js	View		5 chunks	+4 lines, -6 lines	0 comments	Download
M	chrome/browser/resources/gaia_auth/offline.css	View	1	6 chunks	+8 lines, -24 lines	0 comments	Download
M	chrome/browser/resources/gaia_auth/offline.html	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	chrome/browser/resources/gaia_auth/offline.js	View		1 chunk	+6 lines, -11 lines	0 comments	Download
M	chrome/browser/resources/gaia_auth/success.html	View	1	1 chunk	+4 lines, -1 line	0 comments	Download
M	chrome/browser/resources/gaia_auth/success.js	View	1	1 chunk	+4 lines, -2 lines	0 comments	Download

Messages

Total messages: 6 (0 generated)

Expand Messages | Collapse Messages

csilv

lgtm++ w/ nits https://chromiumcodereview.appspot.com/10440061/diff/1/chrome/browser/resources/gaia_auth/main.html File chrome/browser/resources/gaia_auth/main.html (right): https://chromiumcodereview.appspot.com/10440061/diff/1/chrome/browser/resources/gaia_auth/main.html#newcode10 chrome/browser/resources/gaia_auth/main.html:10: frameborder="0"></iframe></body> nit: linefeed before </body> https://chromiumcodereview.appspot.com/10440061/diff/1/chrome/browser/resources/gaia_auth/offline.css ...

8 years, 7 months ago (2012-05-25 23:39:19 UTC) #2

zel

https://chromiumcodereview.appspot.com/10440061/diff/1/chrome/browser/resources/gaia_auth/main.html File chrome/browser/resources/gaia_auth/main.html (right): https://chromiumcodereview.appspot.com/10440061/diff/1/chrome/browser/resources/gaia_auth/main.html#newcode10 chrome/browser/resources/gaia_auth/main.html:10: frameborder="0"></iframe></body> On 2012/05/25 23:39:19, csilv wrote: > nit: linefeed ...

8 years, 7 months ago (2012-05-25 23:46:43 UTC) #3

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/zelidrag@chromium.org/10440061/7001

8 years, 7 months ago (2012-05-25 23:57:31 UTC) #4

commit-bot: I haz the power

Presubmit check for 10440061-7001 failed and returned exit status 1. Running presubmit commit checks ...

8 years, 7 months ago (2012-05-25 23:58:00 UTC) #5

Presubmit check for 10440061-7001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Messages **
Was the CSS checker useful? Send feedback or hate mail to dbeam@chromium.org.

See the JavaScript style guide at
http://www.chromium.org/developers/web-development-style-guide#TOC-JavaScript
and if you have any feedback about the JavaScript PRESUBMIT check, contact
tbreisacher@chromium.org

** Presubmit Warnings **
License must match:
.*? Copyright \(c\) 2012 The Chromium Authors\. All rights reserved\.\n.*? Use
of this source code is governed by a BSD-style license that can be\n.*? found in
the LICENSE file\.(?: \*/)?\n
Found a bad license header in these files:
  chrome/browser/resources/gaia_auth/main.css \
  chrome/browser/resources/gaia_auth/offline.css \
  chrome/browser/resources/gaia_auth/offline.js

chrome/browser/resources/gaia_auth/main.css:
- One selector per line (what not to do: a, b {}).
    html, body {

chrome/browser/resources/gaia_auth/offline.css:
- One selector per line (what not to do: a, b {}).
    body, div, dl, h1, h2, h3, h4, h5, h6, html, img,
    dd, dt, embed, form, object, td, tr, canvas, command, group, mark, meter,
    output, progress, summary, audio, time, video {
    h1, h2, h3, h4, h5, h6 {
    body, html {
    button, input, select, textarea {

- Use rgb() over #hex when not a shade of gray (like #333).
    border: 1px solid #4d90fe; (replace with rgb(77, 144, 254))
    background-color: #4d90fe; (replace with rgb(77, 144, 254))
    background-image: -webkit-gradient(linear,left top,left
bottom,from(#4d90fe),to(#4787ed)); (replace with rgb(77, 144, 254), rgb(71, 135,
237))
    background-image: -webkit-linear-gradient(top,#4d90fe,#4787ed); (replace
with rgb(77, 144, 254), rgb(71, 135, 237))
    background-image: -ms-linear-gradient(top,#4d90fe,#4787ed); (replace with
rgb(77, 144, 254), rgb(71, 135, 237))
    background-image: -o-linear-gradient(top,#4d90fe,#4787ed); (replace with
rgb(77, 144, 254), rgb(71, 135, 237))
    background-image: linear-gradient(top,#4d90fe,#4787ed); (replace with
rgb(77, 144, 254), rgb(71, 135, 237))
    border: 1px solid #3079ed; (replace with rgb(48, 121, 237))
    background-color: #357ae8; (replace with rgb(53, 122, 232))
    background-image: -webkit-linear-gradient(top,#4d90fe,#357ae8); (replace
with rgb(77, 144, 254), rgb(53, 122, 232))
    background-image: linear-gradient(top,#4d90fe,#357ae8); (replace with
rgb(77, 144, 254), rgb(53, 122, 232))
    border: 1px solid #2f5bb7; (replace with rgb(47, 91, 183))
    color: #dd4b39; (replace with rgb(221, 75, 57))
    border: 1px solid #dd4b39; (replace with rgb(221, 75, 57))

- Make all zero length terms (i.e. 0px) 0 unless inside of hsl() or part of
@keyframe.
    -webkit-transition: all 0ms;
    transition: all 0ms;

Found JavaScript style violations in chrome/browser/resources/gaia_auth/main.js:
  line 8: E0220: No docs found for member 'Authenticator.getInstance'
Authenticator.getInstance = function() {
^^^^^^^^^^^^^^^^^^^^^^^^^

Presubmit checks took 5.7s to calculate.

csilv

8 years, 7 months ago (2012-05-26 00:10:44 UTC) #6

https://chromiumcodereview.appspot.com/10440061/diff/1/chrome/browser/resourc...
File chrome/browser/resources/gaia_auth/main.css (right):

https://chromiumcodereview.appspot.com/10440061/diff/1/chrome/browser/resourc...
chrome/browser/resources/gaia_auth/main.css:4: }
add margin: 0; on body as well, webkit default is 8px.

https://chromiumcodereview.appspot.com/10440061/diff/1/chrome/browser/resourc...
File chrome/browser/resources/gaia_auth/main.html (right):

https://chromiumcodereview.appspot.com/10440061/diff/1/chrome/browser/resourc...
chrome/browser/resources/gaia_auth/main.html:10:
frameborder="0"></iframe></body>
On 2012/05/25 23:46:43, zel wrote:
> On 2012/05/25 23:39:19, csilv wrote:
> > nit: linefeed before </body>
> 
> Hmm, isn't that going to change my page design too? I can throw anything else
> than this frame on that page.

It won't change layout as long as it's just whitespace, iframe will get 100%
height and width.  Note my comment in css however about adding margin:0 to body.

Expand Messages | Collapse Messages