‘MakeMy Trip Login’ button doesn't seen properly on http://www.makemytrip.com/
Reported by
dchau...@etouch.net,
Mar 3 2016
|
|||||||||||||
Issue descriptionChrome Version: 50.0.2661.11 (Official Build) d99fa8d200178bce07b02a15d79512e7236088ee-refs/branch-heads/2661@{#51} 32/64-bit. OS: Mac What steps will reproduce the problem? 1. Launch chrome and go to http://www.makemytrip.com/ 2. Click on ‘Login my account’ button and then click on ‘Forgot Password’ link. 3. Now enter the valid ‘Email Id’ in textbox and click on submit button and observe. ‘MakeMy Trip Login’ button is not seen properly. ‘MakeMy Trip Login’ button should seen properly. This is a non-regression issue seen from M-30 series. Note: 1. This issue is not seen on ‘Windows’ OS, will soon update the behaviour of ‘Linux’ OS. 2. This is working fine in Mozilla Firefox browser. Kindly review the attached screencast for reference.
,
Mar 3 2016
Above issue is not reproducible on 'Linux' OS.
,
Mar 3 2016
,
Mar 3 2016
,
Mar 4 2016
,
Mar 9 2016
Are you referring to the grey color of the text, that you expect to be blue? The style for classes .chf_primary_btn, a.chf_primary_btn, in ch.css, includes "color: #fefeff !important;" which is overriding the colors specified elsewhere.
,
Mar 10 2016
With response to comment #6: Text color should be white and background color should be red. Kindly review the attached screen-shot for reference.
,
Mar 10 2016
I can reproduce and have saved a local copy to reduce it.
,
Mar 11 2016
Reduced testcase:
<!doctype HTML>
<a id="target" onclick="toggleColor()" style="-webkit-appearance: button">test</a>
<script>
function toggleColor() {
console.log("toggleColor() called")
target.style.background = "red"
}
</script>
Clicking the link does not turn it red unless you remove -webkit-appearance: button.
This is almost certainly yet another corner case of the textDecorationOrColorChanged optimization in LayoutObject::adjustStyleDifference. There is text, but it doesn't appear that way to the code
because of -webkit-appearance.
,
Mar 11 2016
I think this is a regression from https://src.chromium.org/viewvc/blink?view=rev&revision=169408, as the issue is we don't cache the ua style in cacheUserAgentBorderAndBackground.
,
Mar 11 2016
@esprehn, I didn't bisect back to confirm your patch caused this, but found that the cached ua style was not being set so it seems likely.
,
Mar 11 2016
Yeah this is because cacheUserAgentBorderAndBackground() checks:
if (!style()->hasAppearance())
return;
but the -webkit-appearance from an author rule won't get applied until below that call in StyleResolver::applyMatchedProperties.
I think the fix is to just make webkit-appearance into a high priority property, which it should be, since other properties depend on its value.
That's a simple change, just move it before zoom in Source/core/css/CSSProperties.in
,
Mar 11 2016
Hmm... Safari behaves the same as we do. Their restrictions on appearance as even more restricted, see: https://github.com/WebKit/webkit/blob/52b2434/Source/WebCore/css/StyleResolver.cpp#L1294 https://github.com/WebKit/webkit/blob/52b2434/Source/WebCore/css/StyleResolver.cpp#L1110 where they have a hard coded list of element names: return localName == HTMLNames::inputTag || localName == HTMLNames::textareaTag || localName == HTMLNames::buttonTag || localName == HTMLNames::progressTag || localName == HTMLNames::selectTag || localName == HTMLNames::meterTag || localName == HTMLNames::isindexTag; given that appearance is poorly specced for values that are not "none", I kind of think we probably shouldn't fix this since at least our crazy is sort of in line with WebKit's. meade@ and nainar@, what's your feeling here?
,
Mar 14 2016
Hi @esprehn, If its as easy a fix as you say - why not? It seems like a developer pain. Though it is Mac only. Can't repro on Linux. I'll make the change.
,
Mar 14 2016
Why is this an interop issue? "-webkit-appearance: button" is non-standard and #13 says we behave the same as Safari.
,
Mar 14 2016
Fair enough, my bad.
,
Mar 16 2016
Closing bug as WontFix -> as per verbal discussion with @timloh, better to match Safari given -webkit-appearance is non standard.
,
Mar 16 2016
For context, my understanding is: - On certain form elements (those where html.css specifies -webkit-appearance), setting certain properties (e.g. background-color on <button>) disables the native styling since we don't know how to apply these styles with the native look. - If you set (e.g.) "-webkit-appearance: button; background: red;" on the any of these form elements, we won't use the native styling. Setting a button appearance on a <button> should be a no-op and setting a button appearance on a <progress> is just silly. So the behaviour seems sensible to me. - If you set (e.g.) "-webkit-appearance: button; background: red;" on any other element, we use the native look (we have to pick between native look or red background). I would guess the behaviour wasn't really thought about but just accidental. It makes -webkit-appearance behave differently on form elements to other elements, but... - Chrome and Safari current behave identically in all of these behaviours. The hard-coded list mentioned in comment #13 is exactly the previously mentioned form elements. So changing anything here is just going to make more browsers more inconsistent. |
|||||||||||||
►
Sign in to add a comment |
|||||||||||||
Comment 1 by ranjitkan@chromium.org
, Mar 3 2016