Summary: | RenderTheme should have a function for disabled text color adjustment | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kent Tamura <tkent> | ||||||||||||||
Component: | Forms | Assignee: | yosin | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | tkent, webkit.review.bot | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Bug Depends on: | 54643 | ||||||||||||||||
Bug Blocks: | |||||||||||||||||
Attachments: |
|
Description
Kent Tamura
2011-12-08 17:05:37 PST
Created attachment 118743 [details]
Patch
Comment on attachment 118743 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=118743&action=review > Source/WebCore/ChangeLog:12337 > * css/html.css: > + > (bdi, output): bdi and output should both use -webkit-isolate as the default value for unicode-bidi. Do not modify unrelated ChangeLog entry. > Source/WebCore/rendering/RenderTheme.h:149 > + // Disabled text color for disabled text controls. The comment is not helpful. Please remove it. > Source/WebCore/rendering/RenderThemeChromiumMac.h:62 > + virtual Color disabledTextColor(const Color& textColor, const Color&) const OVERRIDE { return textColor; } > + No reason to make this protected. > Source/WebCore/rendering/RenderThemeChromiumSkia.h:155 > + virtual Color disabledTextColor(const Color& textColor, const Color&) const OVERRIDE { return textColor; } > + ditto. Created attachment 118746 [details]
Patch 2
Comment on attachment 118746 [details] Patch 2 View in context: https://bugs.webkit.org/attachment.cgi?id=118746&action=review > Source/WebCore/rendering/RenderThemeChromiumMac.h:35 > + virtual Color disabledTextColor(const Color& textColor, const Color&) const OVERRIDE { return textColor; } nit: This function can be private. > Source/WebCore/rendering/RenderThemeChromiumSkia.h:61 > + virtual Color disabledTextColor(const Color& textColor, const Color&) const OVERRIDE { return textColor; } dito. Comment on attachment 118746 [details] Patch 2 Clearing flags on attachment: 118746 Committed r102655: <http://trac.webkit.org/changeset/102655> All reviewed patches have been landed. Closing bug. Reopening to attach new patch. Created attachment 118941 [details]
Patch 3
Comment on attachment 118941 [details] Patch 3 View in context: https://bugs.webkit.org/attachment.cgi?id=118941&action=review Your "Patch 2" was already landed. This patch doesn't work. > LayoutTests/ChangeLog:10 > +2011-12-13 Yosifumi Inoue <yosin@chromium.org> > + > + [Forms] Disabled option selected in form > + https://bugs.webkit.org/show_bug.cgi?id=74270 > + > + Reviewed by NOBODY (OOPS!). > + > + * fast/forms/select/menulist-disabled-option-expected.html: Added. > + * fast/forms/select/menulist-disabled-option.html: Added. > + Do not add unrelated ChangeLog. Created attachment 118950 [details]
Patch 4
Comment on attachment 118950 [details]
Patch 4
Please post a patch which can be applied to the latest WebKit revision.
Created attachment 118954 [details]
Patch 5 - Rebase
Comment on attachment 118954 [details] Patch 5 - Rebase View in context: https://bugs.webkit.org/attachment.cgi?id=118954&action=review > Source/WebCore/ChangeLog:16 > + * rendering/RenderTextControl.cpp: > + (WebCore::RenderTextControl::adjustInnerTextStyle): Use RenderTheme::disabledTextColor instead of PLATFORM wraped static function. > + * rendering/RenderTheme.cpp: > + (WebCore::RenderTheme::disabledTextColor): Moved from RenderTextControl.cpp. This method implements for non-Chromium color. > + * rendering/RenderTheme.h: Add new virtual method disabledTextColor. > + * rendering/RenderThemeChromiumMac.h: Implementation of RenderTheme::disabledTextColor for Chrimium Mac. > + * rendering/RenderThemeChromiumSkia.h: Implementation of RenderTheme::disabledTextColor for Chrimium. Please update the file list. Created attachment 118956 [details]
Patch 6 - Rebase
Comment on attachment 118956 [details]
Patch 6 - Rebase
ok
Comment on attachment 118956 [details] Patch 6 - Rebase Clearing flags on attachment: 118956 Committed r102660: <http://trac.webkit.org/changeset/102660> All reviewed patches have been landed. Closing bug. |