Summary: | Add a themeChromiumAndroid.css file for android-specific default styles | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||||
Component: | New Bugs | Assignee: | Eric Seidel (no email) <eric> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, macpherson, menard, satish, tkent, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 66687 | ||||||||
Attachments: |
|
Description
Eric Seidel (no email)
2012-02-13 16:03:47 PST
Created attachment 126856 [details]
Patch
Tkent might be curious to see the styling of the datetime inputs, but I think this can just be a rubber-stamp review, it's very simple. Comment on attachment 126856 [details] Patch > static const RGBA32 defaultTapHighlightColor = 0x6633b5e5; Should this be under "#if ENABLE(TOUCH_EVENTS)" as well? Comment on attachment 126856 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=126856&action=review Ok. > Source/WebCore/rendering/RenderThemeChromiumAndroid.h:50 > private: Missing a blank line above this line. Comment on attachment 126856 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=126856&action=review > Source/WebCore/WebCore.gyp/WebCore.gyp:825 > '../css/themeChromiumSkia.css', # Chromium only. nit: it seems like someone tried to align them, may be good as a drive-by to remove the extra space here. > Source/WebCore/css/themeChromiumAndroid.css:35 > +select[size][multiple] { Is this last selector necessary? It seems to me that any elements having both @size and @multiple also match one of the earlier two selectors. Comment on attachment 126856 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=126856&action=review > Source/WebCore/css/themeChromiumAndroid.css:2 > + * Copyright (C) 2009 Google Inc. All rights reserved. 2012? > Source/WebCore/css/themeChromiumAndroid.css:45 > +input[type="date"], input[type="datetime"], input[type="datetime-local"], input[type="time"], input[type="month"] { > + -webkit-appearance: menulist-button; > +} I understand how they are implemented in Android and it's ok to commit this as is for now. These types inherit a text field type in WebCore/html/*InputType. We should make a better way to switch form control UI. > Source/WebCore/rendering/RenderThemeChromiumAndroid.h:45 > + virtual String extraDefaultStyleSheet(); > > virtual Color systemColor(int cssValidId) const; > > virtual void adjustInnerSpinButtonStyle(CSSStyleSelector*, RenderStyle*, Element*) const; > > + virtual bool delegatesMenuListRendering() const { return true; } > + > +#if ENABLE(TOUCH_EVENTS) > + virtual Color platformTapHighlightColor() const Should append OVERRIDE. Wow! Thanks for all the reviews! Will upload a new patch. (In reply to comment #5) > (From update of attachment 126856 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=126856&action=review > > > Source/WebCore/WebCore.gyp/WebCore.gyp:825 > > '../css/themeChromiumSkia.css', # Chromium only. > > nit: it seems like someone tried to align them, may be good as a drive-by to remove the extra space here. I don't really see any alignment pattern. I can remove the extra space from the chromiumskia one, but I think I'll just leave it: '../css/themeChromium.css', # Chromium only. '../css/themeChromiumAndroid.css', # Chromium only. '../css/themeChromiumLinux.css', # Chromium only. '../css/themeChromiumSkia.css', # Chromium only. > > Source/WebCore/css/themeChromiumAndroid.css:35 > > +select[size][multiple] { > > Is this last selector necessary? It seems to me that any elements having both @size and @multiple also match one of the earlier two selectors. No clue. Created attachment 127408 [details]
Patch for landing
Comment on attachment 127408 [details] Patch for landing Clearing flags on attachment: 127408 Committed r107998: <http://trac.webkit.org/changeset/107998> All reviewed patches have been landed. Closing bug. |